From 870002abc15d61857d8cf34d130bd5b66b2ab317 Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Mon, 10 Aug 2026 15:07:14 +0200 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F(backend)=20remove=20usage=20?= =?UTF-8?q?of=20s3=20for=20document.content=20in=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tehe DocumentFactory was always creating a content and this content was saved on S3. This leads to the creation of huge amount of content in the S3 storage but not necesseraly used in the tests. In order to keep the refactor to remove the usage of content from document.content but from Yhub service, this content is no more generated. It is kept for part of the code not yet refactor like the versionning feature. --- src/backend/core/factories.py | 13 +++- src/backend/core/tests/commands/test_index.py | 14 +++- src/backend/core/tests/conftest.py | 29 +++++--- .../documents/test_api_document_versions.py | 68 ++++++++++++------- .../documents/test_api_documents_duplicate.py | 39 +++++------ .../test_api_documents_formatted_content.py | 26 ++++--- .../core/tests/test_models_documents.py | 4 +- .../tests/test_services_search_indexers.py | 40 +++++++---- 8 files changed, 148 insertions(+), 85 deletions(-) diff --git a/src/backend/core/factories.py b/src/backend/core/factories.py index eeefa8f4b..d918c073f 100644 --- a/src/backend/core/factories.py +++ b/src/backend/core/factories.py @@ -2,6 +2,8 @@ Core application factories """ +import base64 + from django.conf import settings from django.contrib.auth.hashers import make_password @@ -28,6 +30,12 @@ YDOC_HELLO_WORLD_BASE64 = ( "dGV4dENvbG9yAXcHZGVmYXVsdCgA9e7y1Q4eD2JhY2tncm91bmRDb2xvcgF3B2RlZmF1bHQA" ) +# The same document as the raw Yjs update the collaboration server serves, which +# is what a test faking `YHubService.get_ydoc` answers with (see the +# `yhub_content` fixture). The base64 above is the legacy object storage format, +# only the tests still about that storage have a use for it. +YDOC_HELLO_WORLD_UPDATE = base64.b64decode(YDOC_HELLO_WORLD_BASE64) + class UserFactory(factory.django.DjangoModelFactory): """A factory to random users for testing purposes.""" @@ -83,7 +91,10 @@ class DocumentFactory(factory.django.DjangoModelFactory): title = factory.Sequence(lambda n: f"document{n}") excerpt = factory.Sequence(lambda n: f"excerpt{n}") - content = YDOC_HELLO_WORLD_BASE64 + # No content: the collaboration server holds it, and a document built here + # is one it knows nothing of. A test needing a document with content fakes + # what the collaboration server serves for it (`YHubService.get_ydoc`), and + # only the ones about the legacy object storage itself pass `content=`. creator = factory.SubFactory(UserFactory) deleted_at = None link_reach = factory.fuzzy.FuzzyChoice( diff --git a/src/backend/core/tests/commands/test_index.py b/src/backend/core/tests/commands/test_index.py index 1b4a42e6c..48be84b4d 100644 --- a/src/backend/core/tests/commands/test_index.py +++ b/src/backend/core/tests/commands/test_index.py @@ -11,7 +11,9 @@ from django.db import transaction import pytest from core import factories +from core.factories import YDOC_HELLO_WORLD_UPDATE from core.services.search_indexers import FindDocumentIndexer +from core.services.yhub_services import YHubService from core.utils.yjs import base64_yjs_to_text # what the fake collaboration server of the indexer_settings fixture serves @@ -28,7 +30,7 @@ def test_index(): with transaction.atomic(): doc = factories.DocumentFactory() - empty_doc = factories.DocumentFactory(title=None, content="") + empty_doc = factories.DocumentFactory(title=None) no_title_doc = factories.DocumentFactory(title=None) factories.UserDocumentAccessFactory(document=doc, user=user) @@ -41,7 +43,15 @@ def test_index(): str(no_title_doc.path): {"users": [user.sub]}, } - with mock.patch.object(FindDocumentIndexer, "push") as mock_push: + # the empty document is the one the collaboration server holds no content + # for, and it has no title either: nothing to index + def get_ydoc(document): + return None if document.pk == empty_doc.pk else YDOC_HELLO_WORLD_UPDATE + + with ( + mock.patch.object(FindDocumentIndexer, "push") as mock_push, + mock.patch.object(YHubService, "get_ydoc", side_effect=get_ydoc), + ): call_command("index") push_call_args = [call.args[0] for call in mock_push.call_args_list] diff --git a/src/backend/core/tests/conftest.py b/src/backend/core/tests/conftest.py index 9e1ada633..b39af92a7 100644 --- a/src/backend/core/tests/conftest.py +++ b/src/backend/core/tests/conftest.py @@ -32,15 +32,32 @@ def mock_user_teams(): yield mock_teams +@pytest.fixture(name="yhub_content") +def yhub_content_fixture(): + """ + Serve the content of every document, as the collaboration server does. + + It owns the content: a document built by the factories has none in the + database, and what it holds is whatever this fake answers for it. The mock + is yielded, so a test can serve another document (`return_value`), none at + all (`return_value = None`) or a different one per document + (`side_effect`). + """ + with mock.patch.object( + YHubService, "get_ydoc", return_value=factories.YDOC_HELLO_WORLD_UPDATE + ) as mock_get_ydoc: + yield mock_get_ydoc + + @pytest.fixture(name="indexer_settings") def indexer_settings_fixture(settings): """ Setup valid settings for the document indexer. Clear the indexer cache. The indexer reads the content of a document from the collaboration server, - which is faked here: it serves what the factories wrote in the database, so - a document built with `content=""` is one the collaboration server holds no - content for. + which is faked here: it holds the same content for every document, and a + test wanting one without content answers `None` for it (see the + `yhub_content` fixture, this is the same fake). """ # pylint: disable-next=import-outside-toplevel @@ -56,12 +73,8 @@ def indexer_settings_fixture(settings): settings.SEARCH_URL = "http://localhost:8081/api/v1.0/documents/search/" settings.SEARCH_INDEXER_COUNTDOWN = 1 - def get_ydoc(_service, document): - """Answer the raw update the collaboration server would serve.""" - return base64.b64decode(document.content) if document.content else None - with mock.patch.object( - YHubService, "get_ydoc", autospec=True, side_effect=get_ydoc + YHubService, "get_ydoc", return_value=factories.YDOC_HELLO_WORLD_UPDATE ): yield settings diff --git a/src/backend/core/tests/documents/test_api_document_versions.py b/src/backend/core/tests/documents/test_api_document_versions.py index 83b8c7f58..7e5105bb5 100644 --- a/src/backend/core/tests/documents/test_api_document_versions.py +++ b/src/backend/core/tests/documents/test_api_document_versions.py @@ -9,11 +9,23 @@ import pytest from rest_framework.test import APIClient from core import factories, models +from core.factories import YDOC_HELLO_WORLD_BASE64 from core.tests.conftest import TEAM, USER, VIA pytestmark = pytest.mark.django_db +def create_document(**kwargs): + """ + Create a document holding content in the legacy object storage. + + Versions are the versions of that object, so these tests are the ones still + about it: the factories give a document no content anymore, the + collaboration server holds it. + """ + return factories.DocumentFactory(content=YDOC_HELLO_WORLD_BASE64, **kwargs) + + @pytest.mark.parametrize("reach", models.LinkReachChoices.values) @pytest.mark.parametrize("role", models.LinkRoleChoices.values) def test_api_document_versions_list_anonymous(role, reach): @@ -21,7 +33,7 @@ def test_api_document_versions_list_anonymous(role, reach): Anonymous users should not be allowed to list document versions for a document whatever the reach and role. """ - document = factories.DocumentFactory(link_role=role, link_reach=reach) + document = create_document(link_role=role, link_reach=reach) # Accesses and traces for other users should not interfere factories.UserDocumentAccessFactory(document=document) @@ -44,7 +56,7 @@ def test_api_document_versions_list_authenticated_unrelated(reach): client = APIClient() client.force_login(user) - document = factories.DocumentFactory(link_reach=reach) + document = create_document(link_reach=reach) factories.UserDocumentAccessFactory.create_batch(3, document=document) # The versions of another document to which the user is related should not be listed either @@ -70,7 +82,7 @@ def test_api_document_versions_list_authenticated_related_success(via, mock_user client = APIClient() client.force_login(user) - document = factories.DocumentFactory() + document = create_document() if via == USER: models.DocumentAccess.objects.create( document=document, @@ -125,7 +137,7 @@ def test_api_document_versions_list_authenticated_related_pagination( client = APIClient() client.force_login(user) - document = factories.DocumentFactory() + document = create_document() for i in range(3): document.content = f"before {i:d}" document.save() @@ -199,9 +211,9 @@ def test_api_document_versions_list_authenticated_related_pagination_parent( client = APIClient() client.force_login(user) - grand_parent = factories.DocumentFactory() - parent = factories.DocumentFactory(parent=grand_parent) - document = factories.DocumentFactory(parent=parent) + grand_parent = create_document() + parent = create_document(parent=grand_parent) + document = create_document(parent=parent) for i in range(3): document.content = f"before {i:d}" document.save() @@ -270,7 +282,7 @@ def test_api_document_versions_list_exceeds_max_page_size(): client = APIClient() client.force_login(user) - document = factories.DocumentFactory(users=[user]) + document = create_document(users=[user]) document.content = "version 2" document.save() @@ -288,7 +300,7 @@ def test_api_document_versions_retrieve_anonymous(reach): Anonymous users should not be allowed to find specific versions for a document with restricted or authenticated link reach. """ - document = factories.DocumentFactory(link_reach=reach) + document = create_document(link_reach=reach) document.content = "new content" document.save() @@ -314,7 +326,7 @@ def test_api_document_versions_retrieve_authenticated_unrelated(reach): client = APIClient() client.force_login(user) - document = factories.DocumentFactory(link_reach=reach) + document = create_document(link_reach=reach) document.content = "new content" document.save() @@ -340,7 +352,7 @@ def test_api_document_versions_retrieve_authenticated_related(via, mock_user_tea client = APIClient() client.force_login(user) - document = factories.DocumentFactory() + document = create_document() document.content = "new content" document.save() @@ -406,9 +418,9 @@ def test_api_document_versions_retrieve_authenticated_related_parent( client = APIClient() client.force_login(user) - grand_parent = factories.DocumentFactory() - parent = factories.DocumentFactory(parent=grand_parent) - document = factories.DocumentFactory(parent=parent) + grand_parent = create_document() + parent = create_document(parent=grand_parent) + document = create_document(parent=parent) document.content = "new content" document.save() @@ -462,7 +474,7 @@ def test_api_document_versions_retrieve_authenticated_related_parent( def test_api_document_versions_create_anonymous(): """Anonymous users should not be allowed to create document versions.""" - document = factories.DocumentFactory() + document = create_document() response = APIClient().post( f"/api/v1.0/documents/{document.id!s}/versions/", @@ -484,7 +496,7 @@ def test_api_document_versions_create_authenticated_unrelated(): client = APIClient() client.force_login(user) - document = factories.DocumentFactory() + document = create_document() response = client.post( f"/api/v1.0/documents/{document.id!s}/versions/", @@ -506,7 +518,7 @@ def test_api_document_versions_create_authenticated_related(via, mock_user_teams client = APIClient() client.force_login(user) - document = factories.DocumentFactory() + document = create_document() if via == USER: factories.UserDocumentAccessFactory(document=document, user=user) elif via == TEAM: @@ -524,8 +536,10 @@ def test_api_document_versions_create_authenticated_related(via, mock_user_teams def test_api_document_versions_update_anonymous(): """Anonymous users should not be allowed to update a document version.""" - access = factories.UserDocumentAccessFactory() - document = access.document + document = create_document() + factories.UserDocumentAccessFactory(document=document) + # a second version of the object: the first one is the latest, which the + # listing excludes document.content = "new content" document.save() @@ -550,8 +564,10 @@ def test_api_document_versions_update_authenticated_unrelated(): client = APIClient() client.force_login(user) - access = factories.UserDocumentAccessFactory() - document = access.document + document = create_document() + factories.UserDocumentAccessFactory(document=document) + # a second version of the object: the first one is the latest, which the + # listing excludes document.content = "new content" document.save() @@ -559,7 +575,7 @@ def test_api_document_versions_update_authenticated_unrelated(): version_id = document.get_versions_slice()["versions"][0]["version_id"] response = client.put( - f"/api/v1.0/documents/{access.document_id!s}/versions/{version_id:s}/", + f"/api/v1.0/documents/{document.id!s}/versions/{version_id:s}/", {"foo": "bar"}, format="json", ) @@ -577,7 +593,7 @@ def test_api_document_versions_update_authenticated_related(via, mock_user_teams client = APIClient() client.force_login(user) - document = factories.DocumentFactory() + document = create_document() if via == USER: factories.UserDocumentAccessFactory(document=document, user=user) @@ -630,7 +646,7 @@ def test_api_document_versions_delete_authenticated(reach): client = APIClient() client.force_login(user) - document = factories.DocumentFactory(link_reach=reach) + document = create_document(link_reach=reach) document.content = "new content" document.save() @@ -655,7 +671,7 @@ def test_api_document_versions_delete_reader_or_editor(via, role, mock_user_team client = APIClient() client.force_login(user) - document = factories.DocumentFactory() + document = create_document() if via == USER: factories.UserDocumentAccessFactory(document=document, user=user, role=role) elif via == TEAM: @@ -692,7 +708,7 @@ def test_api_document_versions_delete_administrator_or_owner(via, mock_user_team client = APIClient() client.force_login(user) - document = factories.DocumentFactory() + document = create_document() role = random.choice(["administrator", "owner"]) if via == USER: factories.UserDocumentAccessFactory(document=document, user=user, role=role) diff --git a/src/backend/core/tests/documents/test_api_documents_duplicate.py b/src/backend/core/tests/documents/test_api_documents_duplicate.py index 95a982779..7ce23dfda 100644 --- a/src/backend/core/tests/documents/test_api_documents_duplicate.py +++ b/src/backend/core/tests/documents/test_api_documents_duplicate.py @@ -2,7 +2,6 @@ Test file uploads API endpoint for users in impress's core app. """ -import base64 import uuid from io import BytesIO from unittest import mock @@ -19,6 +18,7 @@ from freezegun import freeze_time from rest_framework.test import APIClient from core import factories, models +from core.factories import YDOC_HELLO_WORLD_UPDATE from core.services.yhub_services import ( ServiceUnavailableError as YHubServiceUnavailableError, ) @@ -31,15 +31,19 @@ def mock_yhub_fixture(): """ The content of a document is held by the collaboration server. - It stands for a server holding the very content the factories gave the - documents, which is what an editor connected to it would have saved. + It stands for a server holding content for every document, which is what an + editor connected to it would have saved; the database holds none of it. A + test caring about the content of a given document declares it in + `mock_yhub.contents`, keyed by document id. """ + contents = {} def get_ydoc(document): - return base64.b64decode(document.content) if document.content else None + return contents.get(document.id, YDOC_HELLO_WORLD_UPDATE) with mock.patch("core.api.viewsets.YHubService") as mock_service: mock_service.return_value.get_ydoc.side_effect = get_ydoc + mock_service.contents = contents yield mock_service @@ -119,17 +123,16 @@ def test_api_documents_duplicate_success(index, mock_yhub): ) ydoc["document-store"] = fragment update = ydoc.get_update() - base64_content = base64.b64encode(update).decode("utf-8") # Create documents document = factories.DocumentFactory( id=document_ids[index], - content=base64_content, link_reach="restricted", users=[user, factories.UserFactory()], title="document with an image", attachments=[key for key, _ in image_refs], ) + mock_yhub.contents[document.id] = update factories.DocumentFactory(id=document_ids[(index + 1) % 3]) # Don't create document for third ID to check that it doesn't impact access to attachments @@ -144,7 +147,7 @@ def test_api_documents_duplicate_success(index, mock_yhub): # the content is copied through the collaboration server assert duplicated_document.content is None mock_yhub.return_value.create_ydoc.assert_called_once_with( - duplicated_document, base64.b64decode(document.content) + duplicated_document, update ) assert duplicated_document.creator == user assert duplicated_document.link_reach == "restricted" @@ -247,7 +250,7 @@ def test_api_documents_duplicate_with_accesses_admin(role, mock_yhub): # the content is copied through the collaboration server assert duplicated_document.content is None mock_yhub.return_value.create_ydoc.assert_called_once_with( - duplicated_document, base64.b64decode(document.content) + duplicated_document, YDOC_HELLO_WORLD_UPDATE ) assert duplicated_document.link_reach == document.link_reach assert duplicated_document.link_role == document.link_role @@ -306,7 +309,7 @@ def test_api_documents_duplicate_with_accesses_non_admin(role, mock_yhub): # the content is copied through the collaboration server assert duplicated_document.content is None mock_yhub.return_value.create_ydoc.assert_called_once_with( - duplicated_document, base64.b64decode(document.content) + duplicated_document, YDOC_HELLO_WORLD_UPDATE ) assert duplicated_document.link_reach == document.link_reach assert duplicated_document.link_role == document.link_role @@ -358,7 +361,7 @@ def test_api_documents_duplicate_non_root_document(role, mock_yhub): # the content is copied through the collaboration server assert duplicated_document.content is None mock_yhub.return_value.create_ydoc.assert_called_once_with( - duplicated_document, base64.b64decode(child.content) + duplicated_document, YDOC_HELLO_WORLD_UPDATE ) assert duplicated_document.link_reach == child.link_reach assert duplicated_document.link_role == child.link_role @@ -576,16 +579,15 @@ def test_api_documents_duplicate_with_descendants_and_attachments(mock_yhub): ] ) ydoc["document-store"] = fragment - update = ydoc.get_update() - root_content = base64.b64encode(update).decode("utf-8") + root_update = ydoc.get_update() root = factories.DocumentFactory( id=root_id, users=[(user, "owner")], title="Root with Image", - content=root_content, attachments=[image_key_root], ) + mock_yhub.contents[root.id] = root_update # Create child with different attachment ydoc_child = pycrdt.Doc() @@ -595,17 +597,16 @@ def test_api_documents_duplicate_with_descendants_and_attachments(mock_yhub): ] ) ydoc_child["document-store"] = fragment_child - update_child = ydoc_child.get_update() - child_content = base64.b64encode(update_child).decode("utf-8") + child_update = ydoc_child.get_update() # child - factories.DocumentFactory( + child = factories.DocumentFactory( id=child_id, parent=root, title="Child with Image", - content=child_content, attachments=[image_key_child], ) + mock_yhub.contents[child.id] = child_update # Duplicate with descendants with mock.patch("core.api.viewsets.posthog_capture") as mock_capture: @@ -638,8 +639,8 @@ def test_api_documents_duplicate_with_descendants_and_attachments(mock_yhub): assert duplicated_root.content is None assert dup_child.content is None assert mock_yhub.return_value.create_ydoc.call_args_list == [ - mock.call(duplicated_root, base64.b64decode(root_content)), - mock.call(dup_child, base64.b64decode(child_content)), + mock.call(duplicated_root, root_update), + mock.call(dup_child, child_update), ] diff --git a/src/backend/core/tests/documents/test_api_documents_formatted_content.py b/src/backend/core/tests/documents/test_api_documents_formatted_content.py index 7a800a10f..d57074f00 100644 --- a/src/backend/core/tests/documents/test_api_documents_formatted_content.py +++ b/src/backend/core/tests/documents/test_api_documents_formatted_content.py @@ -2,7 +2,6 @@ Tests for Documents API endpoint in impress's core app: convert """ -import base64 from unittest.mock import patch import pytest @@ -11,6 +10,7 @@ from rest_framework import status from rest_framework.test import APIClient from core import factories +from core.factories import YDOC_HELLO_WORLD_UPDATE from core.services.yhub_services import ( ServiceUnavailableError as YHubServiceUnavailableError, ) @@ -23,15 +23,12 @@ def mock_yhub_fixture(): """ The content of a document is held by the collaboration server. - It stands for a server holding the very content the factories gave the - documents, which is what an editor connected to it would have saved. + It stands for a server holding content for every document, which is what an + editor connected to it would have saved. The documents themselves hold none + in the database, nothing writes it there anymore. """ - - def get_ydoc(document): - return base64.b64decode(document.content) if document.content else None - with patch("core.api.viewsets.YHubService") as mock_service: - mock_service.return_value.get_ydoc.side_effect = get_ydoc + mock_service.return_value.get_ydoc.return_value = YDOC_HELLO_WORLD_UPDATE yield mock_service @@ -58,7 +55,7 @@ def test_api_documents_formatted_content_public(mock_content, reach, role): assert data["title"] == document.title assert data["content"] == {"some": "data"} mock_content.assert_called_once_with( - base64.b64decode(document.content), + YDOC_HELLO_WORLD_UPDATE, "application/vnd.yjs.doc", "application/json", ) @@ -117,7 +114,7 @@ def test_api_documents_formatted_content_not_public( assert data["title"] == document.title assert data["content"] == {"some": "data"} mock_content.assert_called_once_with( - base64.b64decode(document.content), + YDOC_HELLO_WORLD_UPDATE, "application/vnd.yjs.doc", "application/json", ) @@ -147,7 +144,7 @@ def test_api_documents_formatted_content_format(mock_content, content_format, ac assert data["title"] == document.title assert data["content"] == {"some": "data"} mock_content.assert_called_once_with( - base64.b64decode(document.content), "application/vnd.yjs.doc", accept + YDOC_HELLO_WORLD_UPDATE, "application/vnd.yjs.doc", accept ) @@ -188,9 +185,11 @@ def test_api_documents_formatted_content_nonexistent_document(mock_request): @patch("core.services.converter_services.YdocConverter._request") -def test_api_documents_formatted_content_empty_document(mock_request): +def test_api_documents_formatted_content_empty_document(mock_request, mock_yhub): """Test that accessing an empty document returns empty content.""" - document = factories.DocumentFactory(link_reach="public", content="") + document = factories.DocumentFactory(link_reach="public") + # an empty document is one the collaboration server holds nothing for + mock_yhub.return_value.get_ydoc.return_value = None response = APIClient().get( f"/api/v1.0/documents/{document.id!s}/formatted-content/" @@ -212,7 +211,6 @@ def test_api_documents_formatted_content_from_collaboration_server( document = factories.DocumentFactory(link_reach="public") mock_content.return_value = {"some": "data"} # what the collaboration server holds, edited since Django last saw it - mock_yhub.return_value.get_ydoc.side_effect = None mock_yhub.return_value.get_ydoc.return_value = b"\x01\x02edited update" response = APIClient().get( diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index 407e239da..570c44084 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -938,7 +938,7 @@ def test_models_documents_get_versions_slice_pagination(settings): settings.DOCUMENT_VERSIONS_PAGE_SIZE = 4 # Create a document with 7 versions - document = factories.DocumentFactory() + document = factories.DocumentFactory(content=factories.YDOC_HELLO_WORLD_BASE64) for i in range(6): document.content = f"bar{i:d}" document.save() @@ -997,7 +997,7 @@ def test_models_documents_get_versions_slice_min_datetime(): def test_models_documents_version_duplicate(): """A new version should be created in object storage only if the content has changed.""" - document = factories.DocumentFactory() + document = factories.DocumentFactory(content=factories.YDOC_HELLO_WORLD_BASE64) file_key = str(document.pk) response = default_storage.connection.meta.client.list_object_versions( diff --git a/src/backend/core/tests/test_services_search_indexers.py b/src/backend/core/tests/test_services_search_indexers.py index 2dcc972d1..324b5d374 100644 --- a/src/backend/core/tests/test_services_search_indexers.py +++ b/src/backend/core/tests/test_services_search_indexers.py @@ -1,6 +1,5 @@ """Tests for Documents search indexers""" -from base64 import b64decode from functools import partial from json import dumps as json_dumps from unittest.mock import patch @@ -231,7 +230,7 @@ def test_services_search_indexers_serialize_document_deleted(): @pytest.mark.usefixtures("indexer_settings") def test_services_search_indexers_serialize_document_empty(): """Empty documents returns empty content in the serialized json.""" - document = factories.DocumentFactory(content="", title=None) + document = factories.DocumentFactory(title=None) indexer = FindDocumentIndexer() result = indexer.serialize_document(document, "", {}) @@ -366,14 +365,14 @@ def test_services_search_indexers_skip_documents_the_content_of_which_is_unreada """ unreadable, readable = factories.DocumentFactory.create_batch(2) - def get_ydoc(_service, document): + def get_ydoc(document): if document.pk == unreadable.pk: raise ServiceUnavailableError("yhub is unreachable") - return b64decode(document.content) + return factories.YDOC_HELLO_WORLD_UPDATE - # a plain replacement: the indexer_settings fixture already serves the - # content of the documents, this test needs one of them to fail - with patch.object(YHubService, "get_ydoc", get_ydoc): + # the indexer_settings fixture serves content for every document, this + # test needs one of them to fail + with patch.object(YHubService, "get_ydoc", side_effect=get_ydoc): assert FindDocumentIndexer().index() == 1 results = {doc["id"] for doc in mock_push.call_args[0][0]} @@ -388,11 +387,18 @@ def test_services_search_indexers_ignore_empty_documents(mock_push): and only the access data relevant to each batch should be used. """ document = factories.DocumentFactory() - factories.DocumentFactory(content="", title="") + empty = factories.DocumentFactory(title="") empty_title = factories.DocumentFactory(title="") - empty_content = factories.DocumentFactory(content="") + empty_content = factories.DocumentFactory() - assert FindDocumentIndexer().index() == 3 + # a document with no content is one the collaboration server holds none for + def get_ydoc(doc): + if doc.pk in (empty.pk, empty_content.pk): + return None + return factories.YDOC_HELLO_WORLD_UPDATE + + with patch.object(YHubService, "get_ydoc", side_effect=get_ydoc): + assert FindDocumentIndexer().index() == 3 assert mock_push.call_count == 1 @@ -417,10 +423,18 @@ def test_services_search_indexers_skip_empty_batches(mock_push, indexer_settings document = factories.DocumentFactory() - # Only empty docs - factories.DocumentFactory.create_batch(5, content="", title="") + # Only empty docs: no title, and no content in the collaboration server + empty = factories.DocumentFactory.create_batch(5, title="") + empty_ids = {doc.pk for doc in empty} - assert FindDocumentIndexer().index() == 1 + with patch.object( + YHubService, + "get_ydoc", + side_effect=lambda doc: ( + None if doc.pk in empty_ids else factories.YDOC_HELLO_WORLD_UPDATE + ), + ): + assert FindDocumentIndexer().index() == 1 assert mock_push.call_count == 1 results = [doc["id"] for doc in mock_push.call_args[0][0]]