♻️(backend) remove usage of s3 for document.content in tests

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.
This commit is contained in:
Manuel Raynaud
2026-09-01 15:28:56 +02:00
committed by Anthony LC
parent 9665310842
commit 870002abc1
8 changed files with 148 additions and 85 deletions
+12 -1
View File
@@ -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(
+12 -2
View File
@@ -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]
+21 -8
View File
@@ -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
@@ -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)
@@ -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),
]
@@ -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(
@@ -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(
@@ -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]]