✨(backend) improve search indexer service configuration

New SEARCH_INDEXER_CLASS setting to define the indexer service class.
Raise ImpoperlyConfigured errors instead of RuntimeError in index service.

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

Author: joehybird

env.d/development/common

@@ -52,7 +52,7 @@ OIDC_AUTH_REQUEST_EXTRA_PARAMS={"acr_values": "eidas1"}
 # Store OIDC tokens in the session
 OIDC_STORE_ACCESS_TOKEN = True  # Store the access token in the session
 OIDC_STORE_REFRESH_TOKEN = True  # Store the encrypted refresh token in the session
-OIDC_STORE_REFRESH_TOKEN_KEY = AnExampleKeyForDevPurposeOnly
+OIDC_STORE_REFRESH_TOKEN_KEY = ThisIsAnExampleKeyForDevPurposeOnly
 
 # AI
 AI_FEATURE_ENABLED=true

src/backend/core/api/serializers.py

@@ -826,12 +826,4 @@ class MoveDocumentSerializer(serializers.Serializer):
 class FindDocumentSerializer(serializers.Serializer):
     """Serializer for Find search requests"""
 
-    q = serializers.CharField(required=True)
-
-    def validate_q(self, value):
-        """Ensure the text field is not empty."""
-
-        if len(value.strip()) == 0:
-            raise serializers.ValidationError("Text field cannot be empty.")
-
-        return value
+    q = serializers.CharField(required=True, allow_blank=False, trim_whitespace=True)

src/backend/core/api/viewsets.py

@@ -12,7 +12,7 @@
 from django.contrib.postgres.aggregates import ArrayAgg
 from django.contrib.postgres.search import TrigramSimilarity
 from django.core.cache import cache
-from django.core.exceptions import ValidationError
+from django.core.exceptions import ImproperlyConfigured, ValidationError
 from django.core.files.storage import default_storage
 from django.core.validators import URLValidator
 from django.db import connection, transaction
@@ -49,7 +49,7 @@
 from core.services.converter_services import (
     YdocConverter,
 )
-from core.services.search_indexers import FindDocumentIndexer
+from core.services.search_indexers import get_document_indexer_class
 from core.tasks.mail import send_ask_for_access_mail
 from core.utils import extract_attachments, filter_descendants
 
@@ -1047,15 +1047,15 @@ def search(self, request, *args, **kwargs):
         serializer.is_valid(raise_exception=True)
 
         try:
-            indexer = FindDocumentIndexer()
+            indexer = get_document_indexer_class()()
             queryset = indexer.search(
                 text=serializer.validated_data.get("q", ""),
                 user=request.user,
                 token=access_token,
             )
-        except RuntimeError:
+        except ImproperlyConfigured:
             return drf.response.Response(
-                {"detail": "The service is not configured properly."},
+                {"detail": "The service is not properly configured."},
                 status=status.HTTP_401_UNAUTHORIZED,
             )
 

src/backend/core/models.py

@@ -951,7 +951,7 @@ def restore(self):
 
 
 @receiver(signals.post_save, sender=Document)
-def document_post_save(sender, instance, **kwargs):
+def document_post_save(sender, instance, **kwargs):  # pylint: disable=unused-argument
     """
     Asynchronous call to the document indexer at the end of the transaction.
     Note : Within the transaction we can have an empty content and a serialization
@@ -1186,7 +1186,7 @@ def get_abilities(self, user):
 
 
 @receiver(signals.post_save, sender=DocumentAccess)
-def document_access_post_save(sender, instance, created, **kwargs):
+def document_access_post_save(sender, instance, created, **kwargs):  # pylint: disable=unused-argument
     """
     Asynchronous call to the document indexer at the end of the transaction.
     """

src/backend/core/services/search_indexers.py

@@ -3,9 +3,12 @@
 import logging
 from abc import ABC, abstractmethod
 from collections import defaultdict
+from functools import cache
 
 from django.conf import settings
 from django.contrib.auth.models import AnonymousUser
+from django.core.exceptions import ImproperlyConfigured
+from django.utils.module_loading import import_string
 
 import requests
 
@@ -14,18 +17,33 @@
 logger = logging.getLogger(__name__)
 
 
+@cache
+def get_document_indexer_class() -> "BaseDocumentIndexer":
+    """Return the indexer backend class based on the settings."""
+    classpath = settings.SEARCH_INDEXER_CLASS
+
+    if not classpath:
+        raise ImproperlyConfigured(
+            "SEARCH_INDEXER_CLASS must be set in Django settings."
+        )
+
+    try:
+        return import_string(settings.SEARCH_INDEXER_CLASS)
+    except ImportError as err:
+        raise ImproperlyConfigured(
+            f"SEARCH_INDEXER_CLASS setting is not valid : {err}"
+        ) from err
+
+
 def get_batch_accesses_by_users_and_teams(paths):
     """
     Get accesses related to a list of document paths,
     grouped by users and teams, including all ancestor paths.
     """
-    # print("paths: ", paths)
     ancestor_map = utils.get_ancestor_to_descendants_map(
         paths, steplen=models.Document.steplen
     )
     ancestor_paths = list(ancestor_map.keys())
-    # print("ancestor map: ", ancestor_map)
-    # print("ancestor paths: ", ancestor_paths)
 
     access_qs = models.DocumentAccess.objects.filter(
         document__path__in=ancestor_paths
@@ -80,6 +98,24 @@ def __init__(self, batch_size=None):
                 Defaults to settings.SEARCH_INDEXER_BATCH_SIZE.
         """
         self.batch_size = batch_size or settings.SEARCH_INDEXER_BATCH_SIZE
+        self.indexer_url = settings.SEARCH_INDEXER_URL
+        self.indexer_secret = settings.SEARCH_INDEXER_SECRET
+        self.search_url = settings.SEARCH_INDEXER_QUERY_URL
+
+        if not self.indexer_url:
+            raise ImproperlyConfigured(
+                "SEARCH_INDEXER_URL must be set in Django settings."
+            )
+
+        if not self.indexer_secret:
+            raise ImproperlyConfigured(
+                "SEARCH_INDEXER_SECRET must be set in Django settings."
+            )
+
+        if not self.search_url:
+            raise ImproperlyConfigured(
+                "SEARCH_INDEXER_QUERY_URL must be set in Django settings."
+            )
 
     def index(self):
         """
@@ -143,7 +179,7 @@ def search(self, text, user, token):
     @abstractmethod
     def search_query(self, data, token) -> dict:
         """
-        Retreive documents from the Find app API.
+        Retrieve documents from the Find app API.
 
         Must be implemented by subclasses.
         """
@@ -204,16 +240,9 @@ def search_query(self, data, token) -> requests.Response:
         Returns:
             dict: A JSON-serializable dictionary.
         """
-        url = getattr(settings, "SEARCH_INDEXER_QUERY_URL", None)
-
-        if not url:
-            raise RuntimeError(
-                "SEARCH_INDEXER_QUERY_URL must be set in Django settings before search."
-            )
-
         try:
             response = requests.post(
-                url,
+                self.search_url,
                 json=data,
                 headers={"Authorization": f"Bearer {token}"},
                 timeout=10,
@@ -222,7 +251,6 @@ def search_query(self, data, token) -> requests.Response:
             return response.json()
         except requests.exceptions.HTTPError as e:
             logger.error("HTTPError: %s", e)
-            logger.error("Response content: %s", response.text)  # type: ignore
             raise
 
     def format_response(self, data: dict):
@@ -238,27 +266,15 @@ def push(self, data):
         Args:
             data (list): List of document dictionaries.
         """
-        url = getattr(settings, "SEARCH_INDEXER_URL", None)
-        if not url:
-            raise RuntimeError(
-                "SEARCH_INDEXER_URL must be set in Django settings before indexing."
-            )
-
-        secret = getattr(settings, "SEARCH_INDEXER_SECRET", None)
-        if not secret:
-            raise RuntimeError(
-                "SEARCH_INDEXER_SECRET must be set in Django settings before indexing."
-            )
 
         try:
             response = requests.post(
-                url,
+                self.indexer_url,
                 json=data,
-                headers={"Authorization": f"Bearer {secret}"},
+                headers={"Authorization": f"Bearer {self.indexer_secret}"},
                 timeout=10,
             )
             response.raise_for_status()
         except requests.exceptions.HTTPError as e:
             logger.error("HTTPError: %s", e)
-            logger.error("Response content: %s", response.text)
             raise

src/backend/core/tasks/find.py

@@ -1,5 +1,6 @@
 """Trigger document indexation using celery task."""
 
+from functools import partial
 from logging import getLogger
 
 from django.conf import settings
@@ -8,8 +9,8 @@
 
 from core import models
 from core.services.search_indexers import (
-    FindDocumentIndexer,
     get_batch_accesses_by_users_and_teams,
+    get_document_indexer_class,
 )
 
 from impress.celery_app import app
@@ -52,7 +53,7 @@ def document_indexer_task(document_id):
         return
 
     doc = models.Document.objects.get(pk=document_id)
-    indexer = FindDocumentIndexer()
+    indexer = get_document_indexer_class()()
     accesses = get_batch_accesses_by_users_and_teams((doc.path,))
 
     data = indexer.serialize_document(document=doc, accesses=accesses)
@@ -75,11 +76,9 @@ def trigger_document_indexer(document, on_commit=False):
         pass
 
     if on_commit:
-
-        def _aux():
-            trigger_document_indexer(document, on_commit=False)
-
-        transaction.on_commit(_aux)
+        transaction.on_commit(
+            partial(trigger_document_indexer, document, on_commit=False)
+        )
     else:
         key = document_indexer_debounce_key(document.pk)
         countdown = getattr(settings, "SEARCH_INDEXER_COUNTDOWN", 1)

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

@@ -2,6 +2,7 @@
 Unit test for `index` command.
 """
 
+from operator import itemgetter
 from unittest import mock
 
 from django.core.management import call_command
@@ -34,19 +35,18 @@ def test_index():
         str(no_title_doc.path): {"users": [user.sub]},
     }
 
-    def sortkey(d):
-        return d["id"]
-
     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]
 
-        assert len(push_call_args) == 1  # called once but with a batch of docs
-        assert sorted(push_call_args[0], key=sortkey) == sorted(
+        # called once but with a batch of docs
+        mock_push.assert_called_once()
+
+        assert sorted(push_call_args[0], key=itemgetter("id")) == sorted(
             [
                 indexer.serialize_document(doc, accesses),
                 indexer.serialize_document(no_title_doc, accesses),
             ],
-            key=sortkey,
+            key=itemgetter("id"),
         )

src/backend/core/tests/documents/test_api_documents_search.py

@@ -57,7 +57,7 @@ def test_api_documents_search_endpoint_is_none(settings):
     response = APIClient().get("/api/v1.0/documents/search/", data={"q": "alpha"})
 
     assert response.status_code == 401
-    assert response.json() == {"detail": "The service is not configured properly."}
+    assert response.json() == {"detail": "The service is not properly configured."}
 
 
 @responses.activate
@@ -75,6 +75,11 @@ def test_api_documents_search_invalid_params(settings):
     assert response.status_code == 400
     assert response.json() == {"q": ["This field is required."]}
 
+    response = APIClient().get("/api/v1.0/documents/search/", data={"q": "    "})
+
+    assert response.status_code == 400
+    assert response.json() == {"q": ["This field may not be blank."]}
+
 
 @responses.activate
 def test_api_documents_search_format(settings):

src/backend/core/tests/test_models_documents.py

@@ -3,6 +3,7 @@
 """
 # pylint: disable=too-many-lines
 
+from operator import itemgetter
 import random
 import smtplib
 import time
@@ -1432,7 +1433,7 @@ def test_models_documents_post_save_indexer(mock_push, settings):
         factories.UserDocumentAccessFactory(document=doc2, user=user)
         factories.UserDocumentAccessFactory(document=doc3, user=user)
 
-    time.sleep(0.1)  # waits for the end of the tasks
+    time.sleep(0.2)  # waits for the end of the tasks
 
     accesses = {
         str(doc1.path): {"users": [user.sub]},
@@ -1444,16 +1445,13 @@ def test_models_documents_post_save_indexer(mock_push, settings):
 
     indexer = FindDocumentIndexer()
 
-    def sortkey(d):
-        return d["id"]
-
-    assert sorted(data, key=sortkey) == sorted(
+    assert sorted(data, key=itemgetter('id')) == sorted(
         [
             indexer.serialize_document(doc1, accesses),
             indexer.serialize_document(doc2, accesses),
             indexer.serialize_document(doc3, accesses),
         ],
-        key=sortkey,
+        key=itemgetter('id'),
     )
 
     # The debounce counters should be reset