✨(backend) add fallback search & default ordering

Filter deleted documents from visited ones.
Set default ordering to the Find API search call (-updated_at)
BaseDocumentIndexer.search now returns a list of document ids instead of models.
Do not call the indexer in signals when SEARCH_INDEXER_CLASS is not defined
or properly configured.

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

Author: joehybird

src/backend/core/api/serializers.py

@@ -883,3 +883,7 @@ class FindDocumentSerializer(serializers.Serializer):
     """Serializer for Find search requests"""
 
     q = serializers.CharField(required=True, allow_blank=False, trim_whitespace=True)
+    page_size = serializers.IntegerField(
+        required=False, min_value=1, max_value=50, default=20
+    )
+    page = serializers.IntegerField(required=False, min_value=1, default=1)

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 ImproperlyConfigured, ValidationError
+from django.core.exceptions import ValidationError
 from django.core.files.storage import default_storage
 from django.core.validators import URLValidator
 from django.db import connection, transaction
@@ -49,7 +49,10 @@
 from core.services.converter_services import (
     YdocConverter,
 )
-from core.services.search_indexers import get_document_indexer_class
+from core.services.search_indexers import (
+    default_document_indexer,
+    get_visited_document_ids_of,
+)
 from core.tasks.mail import send_ask_for_access_mail
 from core.utils import extract_attachments, filter_descendants
 
@@ -1042,23 +1045,42 @@ def search(self, request, *args, **kwargs):
         The filtering allows full text search through the opensearch indexation app "find".
         """
         access_token = request.session.get("oidc_access_token")
+        user = request.user
 
         serializer = serializers.FindDocumentSerializer(data=request.query_params)
         serializer.is_valid(raise_exception=True)
 
-        try:
-            indexer = get_document_indexer_class()()
-            queryset = indexer.search(
-                text=serializer.validated_data.get("q", ""),
-                user=request.user,
-                token=access_token,
+        indexer = default_document_indexer()
+
+        if not indexer:
+            queryset = self.get_queryset()
+            filterset = DocumentFilter(
+                {"title": serializer.validated_data.get("q", "")}, queryset=queryset
             )
-        except ImproperlyConfigured:
-            return drf.response.Response(
-                {"detail": "The service is not properly configured."},
-                status=status.HTTP_401_UNAUTHORIZED,
+
+            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,
+                },
             )
 
+        queryset = models.Document.objects.all()
+        results = indexer.search(
+            text=serializer.validated_data.get("q", ""),
+            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)
+
         return self.get_response_for_queryset(
             queryset,
             context={

src/backend/core/services/search_indexers.py

@@ -8,6 +8,7 @@
 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
@@ -18,7 +19,23 @@
 
 
 @cache
-def get_document_indexer_class() -> "BaseDocumentIndexer":
+def default_document_indexer():
+    """Returns default indexer service is enabled and properly configured."""
+
+    # For this usecase an empty indexer class is not an issue but a feature.
+    if not getattr(settings, "SEARCH_INDEXER_CLASS", None):
+        logger.info("Document indexer is not configured (see SEARCH_INDEXER_CLASS)")
+        return None
+
+    try:
+        return get_document_indexer_class()()
+    except ImproperlyConfigured as err:
+        logger.error("Document indexer is not properly configured : %s", err)
+        return None
+
+
+@cache
+def get_document_indexer_class():
     """Return the indexer backend class based on the settings."""
     classpath = settings.SEARCH_INDEXER_CLASS
 
@@ -65,7 +82,7 @@ def get_batch_accesses_by_users_and_teams(paths):
     return dict(access_by_document_path)
 
 
-def get_visited_document_ids_of(user):
+def get_visited_document_ids_of(queryset, user):
     """
     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
@@ -74,11 +91,18 @@ def get_visited_document_ids_of(user):
     if isinstance(user, AnonymousUser):
         return []
 
-    qs = models.LinkTrace.objects.filter(user=user).exclude(
-        document__accesses__user=user,
+    qs = models.LinkTrace.objects.filter(user=user)
+
+    docs = (
+        queryset.exclude(accesses__user=user)
+        .filter(
+            deleted_at__isnull=True,
+            ancestors_deleted_at__isnull=True,
+        )
+        .filter(pk__in=Subquery(qs.values("document_id")))
     )
 
-    return list({str(id) for id in qs.values_list("document_id", flat=True)})
+    return list({str(id) for id in docs.values_list("pk", flat=True)})
 
 
 class BaseDocumentIndexer(ABC):
@@ -159,22 +183,41 @@ def push(self, data):
         Must be implemented by subclasses.
         """
 
-    def search(self, text, user, token):
+    # pylint: disable-next=too-many-arguments,too-many-positional-arguments
+    def search(self, text, token, visited=(), page=1, page_size=50):
         """
         Search for documents in Find app.
-        """
-        visited_ids = get_visited_document_ids_of(user)
+        Ensure the same default ordering as "Docs" list : -updated_at
 
+        Returns ids of the documents
+
+        Args:
+            text (str): Text search content.
+            token (str): OIDC Authentication token.
+            visited (list, optional):
+                List of ids of active public documents with LinkTrace
+                Defaults to settings.SEARCH_INDEXER_BATCH_SIZE.
+            page (int, optional):
+                The page number to retrieve.
+                Defaults to 1 if not specified.
+            page_size (int, optional):
+                The number of results to return per page.
+                Defaults to 50 if not specified.
+        """
         response = self.search_query(
             data={
                 "q": text,
-                "visited": visited_ids,
+                "visited": visited,
                 "services": ["docs"],
+                "page_number": page,
+                "page_size": page_size,
+                "order_by": "updated_at",
+                "order_direction": "desc",
             },
             token=token,
         )
 
-        return self.format_response(response)
+        return [d["_id"] for d in response]
 
     @abstractmethod
     def search_query(self, data, token) -> dict:
@@ -184,14 +227,6 @@ def search_query(self, data, token) -> dict:
         Must be implemented by subclasses.
         """
 
-    @abstractmethod
-    def format_response(self, data: dict):
-        """
-        Convert the JSON response from Find app as document queryset.
-
-        Must be implemented by subclasses.
-        """
-
 
 class FindDocumentIndexer(BaseDocumentIndexer):
     """
@@ -253,20 +288,13 @@ def search_query(self, data, token) -> requests.Response:
             logger.error("HTTPError: %s", e)
             raise
 
-    def format_response(self, data: dict):
-        """
-        Retrieve documents ids from Find app response and return a queryset.
-        """
-        return models.Document.objects.filter(pk__in=[d["_id"] for d in data])
-
     def push(self, data):
         """
         Push a batch of documents to the Find backend.
 
         Args:
             data (list): List of document dictionaries.
         """
-
         try:
             response = requests.post(
                 self.indexer_url,

src/backend/core/signals.py

@@ -2,10 +2,14 @@
 Declare and configure the signals for the impress core application
 """
 
+from functools import partial
+
+from django.db import transaction
 from django.db.models import signals
 from django.dispatch import receiver
 
 from . import models
+from .services.search_indexers import default_document_indexer
 from .tasks.find import trigger_document_indexer
 
 
@@ -16,13 +20,14 @@ def document_post_save(sender, instance, **kwargs):  # pylint: disable=unused-ar
     Note : Within the transaction we can have an empty content and a serialization
     error.
     """
-    trigger_document_indexer(instance, on_commit=True)
+    if default_document_indexer() is not None:
+        transaction.on_commit(partial(trigger_document_indexer, instance))
 
 
 @receiver(signals.post_save, sender=models.DocumentAccess)
 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.
     """
-    if not created:
-        trigger_document_indexer(instance.document, on_commit=True)
+    if not created and default_document_indexer() is not None:
+        transaction.on_commit(partial(trigger_document_indexer, instance.document))

src/backend/core/tasks/find.py

@@ -1,11 +1,9 @@
 """Trigger document indexation using celery task."""
 
-from functools import partial
 from logging import getLogger
 
 from django.conf import settings
 from django.core.cache import cache
-from django.db import transaction
 
 from impress.celery_app import app
 
@@ -37,7 +35,7 @@ def decr_counter(key):
 
 @app.task
 def document_indexer_task(document_id):
-    """Send indexation query for a document using celery task."""
+    """Celery Task : Sends indexation query for a document."""
     key = document_indexer_debounce_key(document_id)
 
     # check if the counter : if still up, skip the task. only the last one
@@ -46,6 +44,7 @@ def document_indexer_task(document_id):
         logger.info("Skip document %s indexation", document_id)
         return
 
+    # Prevents some circular imports
     # pylint: disable=import-outside-toplevel
     from core import models  # noqa: PLC0415
     from core.services.search_indexers import (  # noqa: PLC0415
@@ -63,35 +62,27 @@ def document_indexer_task(document_id):
     indexer.push(data)
 
 
-def trigger_document_indexer(document, on_commit=False):
+def trigger_document_indexer(document):
     """
     Trigger indexation task with debounce a delay set by the SEARCH_INDEXER_COUNTDOWN setting.
 
     Args:
         document (Document): The document instance.
-        on_commit (bool): Wait for the end of the transaction before starting the task
-            (some fields may be in wrong state within the transaction)
     """
-
     if document.deleted_at or document.ancestors_deleted_at:
-        pass
-
-    if on_commit:
-        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)
-
-        logger.info(
-            "Add task for document %s indexation in %.2f seconds",
-            document.pk,
-            countdown,
-        )
-
-        # Each time this method is called during the countdown, we increment the
-        # counter and each task decrease it, so the index be run only once.
-        incr_counter(key)
-
-        document_indexer_task.apply_async(args=[document.pk], countdown=countdown)
+        return
+
+    key = document_indexer_debounce_key(document.pk)
+    countdown = getattr(settings, "SEARCH_INDEXER_COUNTDOWN", 1)
+
+    logger.info(
+        "Add task for document %s indexation in %.2f seconds",
+        document.pk,
+        countdown,
+    )
+
+    # Each time this method is called during the countdown, we increment the
+    # counter and each task decrease it, so the index be run only once.
+    incr_counter(key)
+
+    document_indexer_task.apply_async(args=[document.pk], countdown=countdown)

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

@@ -15,6 +15,7 @@
 
 
 @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()

src/backend/core/tests/conftest.py

@@ -24,3 +24,32 @@ def mock_user_teams():
         "core.models.User.teams", new_callable=mock.PropertyMock
     ) as mock_teams:
         yield mock_teams
+
+
+@pytest.fixture(name="indexer_settings")
+def indexer_settings_fixture(settings):
+    """
+    Setup valid settings for the document indexer. Clear the indexer cache.
+    """
+
+    # pylint: disable-next=import-outside-toplevel
+    from core.services.search_indexers import (  # noqa: PLC0415
+        default_document_indexer,
+        get_document_indexer_class,
+    )
+
+    default_document_indexer.cache_clear()
+    get_document_indexer_class.cache_clear()
+
+    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/"
+    )
+
+    yield settings
+
+    # clear cache to prevent issues with other tests
+    default_document_indexer.cache_clear()
+    get_document_indexer_class.cache_clear()