mirror of
https://github.com/suitenumerique/docs.git
synced 2026-09-23 10:05:10 +02:00
🔥(backend) remove Document.content
The content is not managed anymore in the Document object nor the django application. The last piece using it was the versioning API and it has been deleted previously. There is no more reason to keep code related of the content in the backend application. The only remaining code is the `file_key` property in the model. It is kept as a safe guard use by the clean_document management command. If the file is kept on the bucket, then the document will be reseed with it the first time the document will be reopen.
This commit is contained in:
@@ -32,8 +32,8 @@ YDOC_HELLO_WORLD_BASE64 = (
|
||||
|
||||
# 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.
|
||||
# `yhub_content` fixture). The base64 above is what a test expecting the text of
|
||||
# that document reads it from.
|
||||
YDOC_HELLO_WORLD_UPDATE = base64.b64decode(YDOC_HELLO_WORLD_BASE64)
|
||||
|
||||
|
||||
@@ -93,8 +93,7 @@ class DocumentFactory(factory.django.DjangoModelFactory):
|
||||
excerpt = factory.Sequence(lambda n: f"excerpt{n}")
|
||||
# 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=`.
|
||||
# what the collaboration server serves for it (`YHubService.get_ydoc`).
|
||||
creator = factory.SubFactory(UserFactory)
|
||||
deleted_at = None
|
||||
link_reach = factory.fuzzy.FuzzyChoice(
|
||||
|
||||
+10
-67
@@ -4,7 +4,6 @@ Declare and configure the models for the impress core application
|
||||
|
||||
# pylint: disable=too-many-lines
|
||||
|
||||
import hashlib
|
||||
import smtplib
|
||||
import uuid
|
||||
from datetime import timedelta
|
||||
@@ -17,8 +16,6 @@ from django.contrib.postgres.fields import ArrayField
|
||||
from django.contrib.postgres.indexes import GinIndex
|
||||
from django.contrib.sites.models import Site
|
||||
from django.core.cache import cache
|
||||
from django.core.files.base import ContentFile
|
||||
from django.core.files.storage import default_storage
|
||||
from django.core.mail import send_mail
|
||||
from django.db import models, transaction
|
||||
from django.db.models import Count
|
||||
@@ -29,7 +26,6 @@ from django.utils.functional import cached_property
|
||||
from django.utils.translation import get_language, override
|
||||
from django.utils.translation import gettext_lazy as _
|
||||
|
||||
from botocore.exceptions import ClientError
|
||||
from rest_framework.exceptions import ValidationError
|
||||
from timezone_field import TimeZoneField
|
||||
from treebeard.mp_tree import MP_Node, MP_NodeManager, MP_NodeQuerySet
|
||||
@@ -955,7 +951,7 @@ class DocumentManager(MP_NodeManager.from_queryset(DocumentQuerySet)):
|
||||
|
||||
# pylint: disable=too-many-public-methods
|
||||
class Document(MP_Node, BaseModel):
|
||||
"""Pad document carrying the content."""
|
||||
"""Pad document, the content of which lives in the collaboration server."""
|
||||
|
||||
title = models.CharField(_("title"), max_length=255, null=True, blank=True)
|
||||
excerpt = models.TextField(_("excerpt"), max_length=300, null=True, blank=True)
|
||||
@@ -993,8 +989,6 @@ class Document(MP_Node, BaseModel):
|
||||
null=True,
|
||||
)
|
||||
|
||||
_content = None
|
||||
|
||||
# Tree structure
|
||||
alphabet = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
|
||||
steplen = 7 # nb siblings max: 3,521,614,606,208
|
||||
@@ -1033,39 +1027,6 @@ class Document(MP_Node, BaseModel):
|
||||
self._ancestors_link_definition = None
|
||||
self._computed_link_definition = None
|
||||
|
||||
def save(self, *args, **kwargs):
|
||||
"""Write content to object storage only if _content has changed."""
|
||||
super().save(*args, **kwargs)
|
||||
if self._content:
|
||||
self.save_content(self._content)
|
||||
|
||||
def save_content(self, content):
|
||||
"""Save content to object storage."""
|
||||
|
||||
file_key = self.file_key
|
||||
bytes_content = content.encode("utf-8")
|
||||
|
||||
# Attempt to directly check if the object exists using the storage client.
|
||||
try:
|
||||
response = default_storage.connection.meta.client.head_object(
|
||||
Bucket=default_storage.bucket_name, Key=file_key
|
||||
)
|
||||
except ClientError as excpt:
|
||||
# If the error is a 404, the object doesn't exist, so we should create it.
|
||||
if excpt.response["Error"]["Code"] == "404":
|
||||
has_changed = True
|
||||
else:
|
||||
raise
|
||||
else:
|
||||
# Compare the existing ETag with the MD5 hash of the new content.
|
||||
has_changed = (
|
||||
response["ETag"].strip('"') != hashlib.md5(bytes_content).hexdigest() # noqa: S324
|
||||
)
|
||||
|
||||
if has_changed:
|
||||
content_file = ContentFile(bytes_content)
|
||||
default_storage.save(file_key, content_file)
|
||||
|
||||
def is_leaf(self):
|
||||
"""
|
||||
:returns: True if the node is has no children
|
||||
@@ -1083,35 +1044,17 @@ class Document(MP_Node, BaseModel):
|
||||
|
||||
@property
|
||||
def file_key(self):
|
||||
"""Key of the object storage file to which the document content is stored"""
|
||||
"""
|
||||
Key of the legacy object storage file that used to hold the content.
|
||||
|
||||
The collaboration server owns the content now, and Django neither reads
|
||||
nor writes this object any more. The key outlives it: the collaboration
|
||||
server seeds a room from that object on first access and replays its
|
||||
versions to rebuild the history, and `clean_document` purges it so that
|
||||
a document it reset cannot be seeded back from what it left behind.
|
||||
"""
|
||||
return f"{self.key_base}/file"
|
||||
|
||||
@property
|
||||
def content(self):
|
||||
"""Return the json content from object storage if available"""
|
||||
if self._content is None and self.id:
|
||||
try:
|
||||
response = self.get_content_response()
|
||||
except FileNotFoundError, ClientError:
|
||||
pass
|
||||
else:
|
||||
self._content = response["Body"].read().decode("utf-8")
|
||||
return self._content
|
||||
|
||||
@content.setter
|
||||
def content(self, content):
|
||||
"""Cache the content, don't write to object storage yet"""
|
||||
if not isinstance(content, str):
|
||||
raise ValueError("content should be a string.")
|
||||
|
||||
self._content = content
|
||||
|
||||
def get_content_response(self):
|
||||
"""Get the content of the document from object storage"""
|
||||
return default_storage.connection.meta.client.get_object(
|
||||
Bucket=default_storage.bucket_name, Key=self.file_key
|
||||
)
|
||||
|
||||
def get_self_and_ancestors_paths(self):
|
||||
"""
|
||||
Return the paths of the document and of all its ancestors, computed from
|
||||
|
||||
@@ -354,7 +354,6 @@ def test_api_documents_children_create_with_docx_file_success(
|
||||
children = Document.objects.get(pk=response.json()["id"])
|
||||
assert children.title == "My Important Document.docx"
|
||||
# the content is saved by the collaboration server, not by Django
|
||||
assert children.content is None
|
||||
mock_yhub.assert_called_once_with(user=user)
|
||||
mock_yhub.return_value.create_ydoc.assert_called_once_with(children, converted_yjs)
|
||||
|
||||
|
||||
@@ -222,8 +222,7 @@ def test_api_documents_content_updated_rolled_key(yhub_jwks):
|
||||
def test_api_documents_content_updated():
|
||||
"""The collaboration server should be able to refresh the date of a document."""
|
||||
with freeze_time("2026-08-01 12:00:00"):
|
||||
# no content: writing one to S3 under a frozen clock breaks its signature
|
||||
document = factories.DocumentFactory(title="my document", content="")
|
||||
document = factories.DocumentFactory(title="my document")
|
||||
|
||||
with freeze_time("2026-08-06 12:00:00"):
|
||||
response = APIClient().post(
|
||||
|
||||
@@ -219,7 +219,6 @@ def test_api_documents_create_for_owner_existing(mock_convert_md, mock_yhub):
|
||||
|
||||
assert document.title == "My Document"
|
||||
# the content is saved by the collaboration server, not by Django
|
||||
assert document.content is None
|
||||
mock_yhub.return_value.create_ydoc.assert_called_once_with(
|
||||
document, CONVERTED_CONTENT
|
||||
)
|
||||
@@ -289,7 +288,6 @@ def test_api_documents_create_for_owner_new_user(mock_convert_md, mock_yhub):
|
||||
|
||||
assert document.title == "My Document"
|
||||
# the content is saved by the collaboration server, not by Django
|
||||
assert document.content is None
|
||||
mock_yhub.return_value.create_ydoc.assert_called_once_with(
|
||||
document, CONVERTED_CONTENT
|
||||
)
|
||||
@@ -401,7 +399,6 @@ def test_api_documents_create_for_owner_existing_user_email_no_sub_with_fallback
|
||||
|
||||
assert document.title == "My Document"
|
||||
# the content is saved by the collaboration server, not by Django
|
||||
assert document.content is None
|
||||
mock_yhub.return_value.create_ydoc.assert_called_once_with(
|
||||
document, CONVERTED_CONTENT
|
||||
)
|
||||
@@ -503,7 +500,6 @@ def test_api_documents_create_for_owner_new_user_no_sub_no_fallback_allow_duplic
|
||||
|
||||
assert document.title == "My Document"
|
||||
# the content is saved by the collaboration server, not by Django
|
||||
assert document.content is None
|
||||
mock_yhub.return_value.create_ydoc.assert_called_once_with(
|
||||
document, CONVERTED_CONTENT
|
||||
)
|
||||
|
||||
@@ -76,10 +76,9 @@ def test_api_documents_create_with_docx_file_success(mock_convert, mock_yhub, se
|
||||
assert response.status_code == 201
|
||||
document = Document.objects.get()
|
||||
assert document.title == "My Important Document.docx"
|
||||
# the content is saved by the collaboration server, not by Django
|
||||
assert document.content is None
|
||||
assert document.accesses.filter(role="owner", user=user).exists()
|
||||
|
||||
# the content is saved by the collaboration server, not by Django
|
||||
mock_yhub.assert_called_once_with(user=user)
|
||||
mock_yhub.return_value.create_ydoc.assert_called_once_with(document, converted_yjs)
|
||||
|
||||
@@ -176,10 +175,9 @@ def test_api_documents_create_with_markdown_file_success(
|
||||
assert response.status_code == 201
|
||||
document = Document.objects.get()
|
||||
assert document.title == "readme.md"
|
||||
# the content is saved by the collaboration server, not by Django
|
||||
assert document.content is None
|
||||
assert document.accesses.filter(role="owner", user=user).exists()
|
||||
|
||||
# the content is saved by the collaboration server, not by Django
|
||||
mock_yhub.return_value.create_ydoc.assert_called_once_with(document, converted_yjs)
|
||||
|
||||
# Verify the converter was called correctly
|
||||
@@ -383,7 +381,6 @@ def test_api_documents_create_without_file_still_works():
|
||||
assert response.status_code == 201
|
||||
document = Document.objects.get()
|
||||
assert document.title == "Regular document without file"
|
||||
assert document.content is None
|
||||
assert document.accesses.filter(role="owner", user=user).exists()
|
||||
|
||||
mock_capture.assert_called_once_with(
|
||||
@@ -465,7 +462,6 @@ def test_api_documents_create_with_file_preserves_content_format(
|
||||
|
||||
# The update is sent untouched, it is not base64 encoded on the way
|
||||
mock_yhub.return_value.create_ydoc.assert_called_once_with(document, converted_yjs)
|
||||
assert document.content is None
|
||||
|
||||
# The successful conversion should be tracked in PostHog
|
||||
mock_capture.assert_any_call(
|
||||
|
||||
@@ -147,7 +147,6 @@ def test_api_documents_duplicate_success(index, mock_yhub):
|
||||
duplicated_document = models.Document.objects.get(id=response.json()["id"])
|
||||
assert duplicated_document.title == "Copy of document with an image"
|
||||
# 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, update
|
||||
)
|
||||
@@ -274,7 +273,6 @@ def test_api_documents_duplicate_with_accesses_admin(role, mock_yhub):
|
||||
duplicated_document = models.Document.objects.get(id=response.json()["id"])
|
||||
assert duplicated_document.title == "Copy of document with accesses"
|
||||
# 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, YDOC_HELLO_WORLD_UPDATE
|
||||
)
|
||||
@@ -333,7 +331,6 @@ def test_api_documents_duplicate_with_accesses_non_admin(role, mock_yhub):
|
||||
duplicated_document = models.Document.objects.get(id=response.json()["id"])
|
||||
assert duplicated_document.title == "Copy of document with accesses"
|
||||
# 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, YDOC_HELLO_WORLD_UPDATE
|
||||
)
|
||||
@@ -385,7 +382,6 @@ def test_api_documents_duplicate_non_root_document(role, mock_yhub):
|
||||
duplicated_document = models.Document.objects.get(id=response.json()["id"])
|
||||
assert duplicated_document.title == "Copy of document with accesses"
|
||||
# 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, YDOC_HELLO_WORLD_UPDATE
|
||||
)
|
||||
@@ -662,8 +658,6 @@ def test_api_documents_duplicate_with_descendants_and_attachments(mock_yhub):
|
||||
assert dup_child.attachments == [image_key_child]
|
||||
|
||||
# the content of the whole subtree is copied through the collaboration server
|
||||
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, root_update),
|
||||
mock.call(dup_child, child_update),
|
||||
|
||||
@@ -302,10 +302,9 @@ def test_external_api_documents_create_with_markdown_file_success(
|
||||
document = models.Document.objects.get(id=data["id"])
|
||||
|
||||
assert document.title == "readme.md"
|
||||
# the content is saved by the collaboration server, not by Django
|
||||
assert document.content is None
|
||||
assert document.accesses.filter(role="owner", user=user_specific_sub).exists()
|
||||
|
||||
# the content is saved by the collaboration server, not by Django
|
||||
mock_yhub.assert_called_once_with(user=user_specific_sub)
|
||||
mock_yhub.return_value.create_ydoc.assert_called_once_with(document, converted_yjs)
|
||||
|
||||
|
||||
@@ -12,7 +12,6 @@ from django.contrib.auth.models import AnonymousUser
|
||||
from django.core import mail
|
||||
from django.core.cache import cache
|
||||
from django.core.exceptions import ValidationError
|
||||
from django.core.files.storage import default_storage
|
||||
from django.test.utils import override_settings
|
||||
from django.utils import timezone
|
||||
|
||||
@@ -908,34 +907,6 @@ def test_models_document_get_abilities_ai_access_public(is_authenticated, reach)
|
||||
assert abilities["ai_translate"] == is_authenticated
|
||||
|
||||
|
||||
def test_models_documents_version_duplicate():
|
||||
"""A new version should be created in object storage only if the content has changed."""
|
||||
document = factories.DocumentFactory(content=factories.YDOC_HELLO_WORLD_BASE64)
|
||||
|
||||
file_key = str(document.pk)
|
||||
response = default_storage.connection.meta.client.list_object_versions(
|
||||
Bucket=default_storage.bucket_name, Prefix=file_key
|
||||
)
|
||||
assert len(response["Versions"]) == 1
|
||||
|
||||
# Save again with the same content
|
||||
document.save()
|
||||
|
||||
response = default_storage.connection.meta.client.list_object_versions(
|
||||
Bucket=default_storage.bucket_name, Prefix=file_key
|
||||
)
|
||||
assert len(response["Versions"]) == 1
|
||||
|
||||
# Save modified content
|
||||
document.content = "new content"
|
||||
document.save()
|
||||
|
||||
response = default_storage.connection.meta.client.list_object_versions(
|
||||
Bucket=default_storage.bucket_name, Prefix=file_key
|
||||
)
|
||||
assert len(response["Versions"]) == 2
|
||||
|
||||
|
||||
def test_models_documents__email_invitation__success():
|
||||
"""
|
||||
The email invitation is sent successfully.
|
||||
|
||||
@@ -44,14 +44,12 @@ def test_commands_create_demo(collaboration_server):
|
||||
assert models.Document.objects.count() >= 10
|
||||
assert models.DocumentAccess.objects.count() > 10
|
||||
|
||||
# every document was seeded with its content in the collaboration server,
|
||||
# and nothing was written to the object storage
|
||||
# every document was seeded with its content in the collaboration server
|
||||
assert collaboration_server.call_count == 10
|
||||
seeded = {call.args[0].id for call in collaboration_server.call_args_list}
|
||||
assert seeded == set(models.Document.objects.values_list("id", flat=True))
|
||||
for call in collaboration_server.call_args_list:
|
||||
document, update = call.args
|
||||
assert document.content is None
|
||||
|
||||
# the structure BlockNote stores, so the editor opens a real document
|
||||
xml = yjs_to_xml(update)
|
||||
|
||||
Reference in New Issue
Block a user