fix: make the Typesense search backend usable

src/forum/search/typesense.py

@@ -2,6 +2,7 @@
 Typesense backend for searching comments and threads.
 """
 
+import math
 from typing import Any, Optional, cast
 
 from bs4 import BeautifulSoup
@@ -23,6 +24,11 @@
 
 _TYPESENSE_CLIENT: Client | None = None
 
+# Typesense rejects any search with per_page above 250 (HTTP 422), so a deep
+# search for FORUM_MAX_DEEP_SEARCH_COMMENT_COUNT hits has to be paginated.
+# https://typesense.org/docs/30.2/api/search.html#pagination-parameters
+TYPESENSE_MAX_PER_PAGE = 250
+
 
 def get_typesense_client() -> Client:
     """
@@ -169,6 +175,7 @@ def build_search_parameters(
     course_id: str | None,
     context: str,
     commentable_ids: list[str] | None,
+    page: int = 1,
 ) -> SearchParameters:
     """
     Build Typesense search parameters for searching the index.
@@ -180,7 +187,8 @@ def build_search_parameters(
 
     if commentable_ids:
         safe_ids = ", ".join(quote_filter_value(value) for value in commentable_ids)
-        filters.append(f"commentable_ids:[{safe_ids}]")
+        # The field is `commentable_id`, singular, as declared in collection_schema().
+        filters.append(f"commentable_id:[{safe_ids}]")
 
     if course_id:
         filters.append(f"course_id:={quote_filter_value(course_id)}")
@@ -189,7 +197,8 @@ def build_search_parameters(
         "q": search_text,
         "query_by": "text",
         "filter_by": " && ".join(filters),
-        "per_page": FORUM_MAX_DEEP_SEARCH_COMMENT_COUNT,
+        "per_page": TYPESENSE_MAX_PER_PAGE,
+        "page": page,
     }
 
 
@@ -408,18 +417,26 @@ def get_thread_ids(
         Retrieve thread IDs based on search criteria.
         """
         client = get_typesense_client()
+        collection = client.collections[collection_name()]
 
-        params = build_search_parameters(
-            search_text=search_text,
-            course_id=course_id,
-            context=context,
-            commentable_ids=commentable_ids,
+        thread_ids: set[str] = set()
+        page_count = math.ceil(
+            FORUM_MAX_DEEP_SEARCH_COMMENT_COUNT / TYPESENSE_MAX_PER_PAGE
         )
+        for page in range(1, page_count + 1):
+            params = build_search_parameters(
+                search_text=search_text,
+                course_id=course_id,
+                context=context,
+                commentable_ids=commentable_ids,
+                page=page,
+            )
+            results = collection.documents.search(params)
+            hits = cast(list[dict[str, Any]], results.get("hits", []))
+            thread_ids.update(hit["document"]["thread_id"] for hit in hits)
+            if len(hits) < TYPESENSE_MAX_PER_PAGE:
+                break
 
-        results = client.collections[collection_name()].documents.search(params)
-        thread_ids: set[str] = {
-            hit["document"]["thread_id"] for hit in results.get("hits", [])  # type: ignore
-        }
         return list(thread_ids)
 
     def get_suggested_text(self, search_text: str) -> Optional[str]:

src/forum/settings/test.py

@@ -82,4 +82,4 @@ def root(*args: str) -> str:
 
 TYPESENSE_COLLECTION_PREFIX = "forum_unittest_prefix_"
 TYPESENSE_API_KEY = "example-typesense-api-key"
-TYPESENSE_URLS = ["http://0.0.0.0:8108"]
+TYPESENSE_URLS = ["http://localhost:5108"]

tests/e2e/docker-compose.yml

@@ -17,3 +17,10 @@ services:
             - 127.0.0.1:5700:7700
         environment:
             MEILI_MASTER_KEY: "MEILISEARCH_MASTER_KEY"
+
+    typesense:
+        # https://hub.docker.com/r/typesense/typesense/tags
+        image: docker.io/typesense/typesense:30.2
+        ports:
+            - 127.0.0.1:5108:8108
+        command: "--data-dir /tmp --api-key=example-typesense-api-key"

tests/e2e/test_search_typesense.py

@@ -0,0 +1,134 @@
+"""
+Typesense end-to-end tests.
+
+These run against a real Typesense server, which the mocked unit tests in
+tests/test_typesense.py cannot substitute for: both of the bugs these tests
+cover were rejections from the server, invisible to a mock.
+"""
+
+import typing as t
+
+from django.test import override_settings
+import pytest
+
+from forum.search import typesense
+
+pytestmark = pytest.mark.django_db
+
+COURSE_ID = "course-v1:Arbisoft+SE002+2024_S2"
+
+
+@pytest.fixture(autouse=True)
+def configure_typesense_search_backend() -> t.Generator[t.Any, t.Any, t.Any]:
+    """Configure Django to use Typesense as a search backend."""
+    with override_settings(
+        FORUM_SEARCH_BACKEND="forum.search.typesense.TypesenseBackend"
+    ):
+        yield
+
+
+@pytest.fixture(autouse=True)
+def typesense_cleanup() -> None:
+    """Start each test from an empty collection."""
+    typesense.TypesenseIndexBackend().initialize_indices(force_new_index=True)
+
+
+def index_threads(threads: list[dict[str, t.Any]]) -> None:
+    """Bulk-index thread documents, then confirm Typesense accepted every one."""
+    client = typesense.get_typesense_client()
+    response = client.collections[typesense.collection_name()].documents.import_(
+        [typesense.document_from_thread(thread["id"], thread) for thread in threads],
+        {"action": "upsert"},
+    )
+    assert all(result["success"] for result in response), response
+
+
+def test_initialize_indices() -> None:
+    index_backend = typesense.TypesenseIndexBackend()
+    index_backend.initialize_indices()
+    # raises AssertionError if the collection on the server does not match the schema
+    index_backend.validate_indices()
+
+
+def test_insert_document(
+    patched_get_backend: t.Any, user_data: tuple[str, str]
+) -> None:
+    index_backend = typesense.TypesenseIndexBackend()
+    index_backend.initialize_indices()
+
+    backend = patched_get_backend()
+    user_id, _ = user_data
+    comment_thread_id = backend.create_thread(
+        {
+            "title": "title",
+            "body": "Hello World!",
+            "pinned": False,
+            "author_id": user_id,
+            "course_id": COURSE_ID,
+            "commentable_id": "66b4e0440dead7001deb948b",
+            "author_username": "Faraz",
+        }
+    )
+
+    index_backend.refresh_indices()
+    thread_backend = typesense.TypesenseThreadSearchBackend()
+    assert thread_backend.get_thread_ids("course", [], "hello") == [comment_thread_id]
+
+
+def test_search_filtered_by_commentable_id() -> None:
+    """
+    Scoping a search to discussion topics has to filter on the field name the
+    collection actually declares, or Typesense rejects the search with HTTP 400.
+    """
+    index_threads(
+        [
+            {
+                "id": index,
+                "course_id": COURSE_ID,
+                "commentable_id": commentable_id,
+                "context": "course",
+                "title": "searchable thread",
+                "body": "<p>Hello World!</p>",
+            }
+            for index, commentable_id in enumerate(["week-1", "week-1", "week-2"])
+        ]
+    )
+
+    thread_backend = typesense.TypesenseThreadSearchBackend()
+    assert sorted(
+        thread_backend.get_thread_ids(
+            "course",
+            [],
+            "searchable",
+            commentable_ids=["week-1"],
+            course_id=COURSE_ID,
+        )
+    ) == ["0", "1"]
+
+
+def test_deep_search_past_the_per_page_limit() -> None:
+    """
+    A deep search asks for FORUM_MAX_DEEP_SEARCH_COMMENT_COUNT hits, which is far
+    more than Typesense will return in one page — it rejects any per_page above
+    250 with HTTP 422. All of the matches still have to come back.
+    """
+    thread_count = typesense.TYPESENSE_MAX_PER_PAGE * 2 + 1
+    index_threads(
+        [
+            {
+                "id": index,
+                "course_id": COURSE_ID,
+                "commentable_id": "week-1",
+                "context": "course",
+                "title": "searchable thread",
+                "body": f"<p>Hello World {index}!</p>",
+            }
+            for index in range(thread_count)
+        ]
+    )
+
+    thread_backend = typesense.TypesenseThreadSearchBackend()
+    thread_ids = thread_backend.get_thread_ids(
+        "course", [], "searchable", commentable_ids=["week-1"], course_id=COURSE_ID
+    )
+    assert len(thread_ids) == thread_count

tests/test_typesense.py

@@ -100,16 +100,90 @@ def test_search_threads(mock_get_client: Mock) -> None:
     expected_params = {
         "q": "thoughts",
         "query_by": "text",
-        "filter_by": "context:`course` && commentable_ids:[`4`, `7[||`] "
+        # `commentable_id` is singular: it must match the field name in collection_schema()
+        "filter_by": "context:`course` && commentable_id:[`4`, `7[||`] "
         "&& course_id:=`course-v1:OpenedX+DemoX+DemoCourse`",
-        "per_page": constants.FORUM_MAX_DEEP_SEARCH_COMMENT_COUNT,
+        "per_page": typesense.TYPESENSE_MAX_PER_PAGE,
+        "page": 1,
     }
+    # a short page means the results are exhausted, so only one request is made
     mock_search.assert_called_once_with(expected_params)
 
     # suggested text is not supported; always returns None
     assert backend.get_suggested_text("foo") is None
 
 
+def test_per_page_within_typesense_limit() -> None:
+    """Typesense returns HTTP 422 for per_page above 250."""
+    assert typesense.TYPESENSE_MAX_PER_PAGE <= 250
+
+
+@patch("forum.search.typesense.get_typesense_client")
+def test_search_threads_paginates(mock_get_client: Mock) -> None:
+    """
+    A full page means there may be more, so the backend walks pages until it has
+    covered FORUM_MAX_DEEP_SEARCH_COMMENT_COUNT hits or a page comes back short.
+    """
+    mock_client = MagicMock()
+    mock_get_client.return_value = mock_client
+    mock_search = mock_client.collections[
+        "forum_unittest_prefix_forum"
+    ].documents.search
+
+    full_page = {
+        "hits": [
+            {"document": {"thread_id": f"T{index}"}}
+            for index in range(typesense.TYPESENSE_MAX_PER_PAGE)
+        ]
+    }
+    short_page = {"hits": [{"document": {"thread_id": "LAST"}}]}
+    mock_search.side_effect = [full_page, short_page]
+
+    backend = typesense.TypesenseThreadSearchBackend()
+    thread_ids = backend.get_thread_ids(
+        context="course",
+        group_ids=[],
+        search_text="thoughts",
+        commentable_ids=None,
+        course_id=None,
+    )
+
+    assert len(thread_ids) == typesense.TYPESENSE_MAX_PER_PAGE + 1
+    assert "LAST" in thread_ids
+    assert [call.args[0]["page"] for call in mock_search.call_args_list] == [1, 2]
+
+
+@patch("forum.search.typesense.get_typesense_client")
+def test_search_threads_stops_at_deep_search_budget(mock_get_client: Mock) -> None:
+    """Full pages all the way down still stop at FORUM_MAX_DEEP_SEARCH_COMMENT_COUNT."""
+    mock_client = MagicMock()
+    mock_get_client.return_value = mock_client
+    mock_search = mock_client.collections[
+        "forum_unittest_prefix_forum"
+    ].documents.search
+    mock_search.return_value = {
+        "hits": [
+            {"document": {"thread_id": f"T{index}"}}
+            for index in range(typesense.TYPESENSE_MAX_PER_PAGE)
+        ]
+    }
+
+    backend = typesense.TypesenseThreadSearchBackend()
+    backend.get_thread_ids(
+        context="course",
+        group_ids=[],
+        search_text="thoughts",
+        commentable_ids=None,
+        course_id=None,
+    )
+
+    expected_pages = (
+        constants.FORUM_MAX_DEEP_SEARCH_COMMENT_COUNT
+        // typesense.TYPESENSE_MAX_PER_PAGE
+    )
+    assert mock_search.call_count == expected_pages
+
+
 @patch("forum.search.typesense.get_typesense_client")
 def test_index_comment_document(mock_get_client: Mock) -> None:
     mock_client = MagicMock()