mirror of
https://github.com/suitenumerique/docs.git
synced 2026-09-11 04:07:53 +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:
+17
-3
@@ -119,9 +119,8 @@ and this project adheres to
|
||||
endpoints since the history panel started reading the collaboration server's
|
||||
`activity` and `changeset` routes instead. Gone with them:
|
||||
`Document.get_versions_slice`, `Document.delete_version`, the `version_id`
|
||||
argument of
|
||||
`get_content_response`, the `DOCUMENT_VERSIONS_PAGE_SIZE` setting, and the
|
||||
`versions_retrieve` and `versions_destroy` abilities
|
||||
argument of `get_content_response`, the `DOCUMENT_VERSIONS_PAGE_SIZE`
|
||||
setting, and the `versions_retrieve` and `versions_destroy` abilities
|
||||
|
||||
The `versions_list` ability stays and keeps its meaning — may this user see
|
||||
this document's history — now as a pure gate rather than the permission for
|
||||
@@ -129,6 +128,21 @@ and this project adheres to
|
||||
collaboration server reads it to decide whether to ask this backend when the
|
||||
caller's access began, which is where the history it serves starts
|
||||
|
||||
- 🔥(backend) remove `Document.content`. The collaboration server has owned the
|
||||
content of the documents since the migration, and the version endpoints
|
||||
removed above were the last thing in Django that read the legacy `{id}/file`
|
||||
object; nothing had written it for as long. Gone with the property: its
|
||||
setter, `Document.save_content`, `Document.get_content_response` and the
|
||||
`save()` override that existed only to write the content — a document is
|
||||
saved by `Model.save` alone now, and creating one no longer costs a `HEAD`
|
||||
and a `PUT` against the object storage
|
||||
|
||||
The objects are untouched and must stay: the collaboration server seeds a
|
||||
room from `{id}/file` on first access (`SOFT_MIGRATION`) and replays its
|
||||
versions to rebuild the history. `Document.file_key` stays with them, and
|
||||
`clean_document` goes on purging that object so that a document it reset
|
||||
cannot be seeded back from what it left behind
|
||||
|
||||
### Fixed
|
||||
|
||||
- 🐛(frontend) stop reconnecting to the collaboration server when it has refused
|
||||
|
||||
+31
-5
@@ -85,8 +85,9 @@ upgrade, in the order they are done, and end with the API changes.
|
||||
2. Then backfill the corpus, which the lazy seeding never finishes on its own
|
||||
— a document nobody opens stays in S3 forever. `python manage.py
|
||||
migrate_documents` hands every document to the collaboration server, which
|
||||
replays its **full** S3 version history rather than its last snapshot, so
|
||||
`/documents/{id}/versions/` and the history the editor shows agree. It is
|
||||
replays its **full** S3 version history rather than its last snapshot,
|
||||
so the history the editor shows reaches back to the document's first save
|
||||
instead of beginning the day it reached the collaboration server. It is
|
||||
bounded (`--concurrency`, `--rate`, `--limit`, `--created-before`),
|
||||
resumable and safe to re-run: what became of every document is recorded in
|
||||
the new `impress_document_migration` table, a document the server refused
|
||||
@@ -98,9 +99,18 @@ upgrade, in the order they are done, and end with the API changes.
|
||||
migrated — but an unmigrated document then opens as an *empty* room over
|
||||
content that is alive in S3.
|
||||
|
||||
Keep the media bucket, its objects and its versioning either way:
|
||||
`/documents/{id}/versions/` still serves the version history from there, and
|
||||
the full migration replays it.
|
||||
The media bucket itself is not going anywhere: the attachments of the
|
||||
documents live in it, under `{document-id}/attachments/`, and the backend
|
||||
goes on uploading them, signing urls for them and scanning them through the
|
||||
S3 API. What this upgrade adds to it is temporary, and has to be kept either
|
||||
way: the legacy `{document-id}/file` objects **and their versions**. Django
|
||||
has let go of that key — it neither reads nor writes it anymore, and the
|
||||
endpoints that used to serve its versions are removed below — but the two
|
||||
steps above are what read it now, the lazy seed taking its newest version and
|
||||
the backfill replaying every one of them. They are the whole record of what
|
||||
the documents were before the collaboration server, so leave the bucket
|
||||
versioned and those objects in place until the backfill has covered the
|
||||
corpus; there is no way back from deleting them.
|
||||
|
||||
- ⚠️ **The websocket url changed**, and so does what `/collaboration/` is
|
||||
routed to. The room is appended by the client, and the last segment of the
|
||||
@@ -242,6 +252,22 @@ upgrade, in the order they are done, and end with the API changes.
|
||||
it. `/api/v1.0/documents/{document_id}/formatted-content/` is not affected.
|
||||
The `CONTENT_METADATA_CACHE_TIMEOUT` setting only tuned the cache of the
|
||||
removed `GET` and is no longer read, you can drop it from your configuration.
|
||||
- The endpoints `GET /api/v1.0/documents/{document_id}/versions/` and `GET,
|
||||
DELETE /api/v1.0/documents/{document_id}/versions/{version_id}/` are removed.
|
||||
They listed and served the S3 object versions of `{document_id}/file`, the
|
||||
key the content of a document used to be stored under; nothing has written it
|
||||
since the content moved to the collaboration server, so what they listed was
|
||||
frozen at each document's migration date. The editor reads the history from
|
||||
the collaboration server instead. If you integrate with Docs, stop calling
|
||||
them: the `versions_retrieve` and `versions_destroy` abilities disappear from
|
||||
the document payload along with them, and the `DOCUMENT_VERSIONS_PAGE_SIZE`
|
||||
setting is no longer read, you can drop it from your configuration.
|
||||
|
||||
The `versions_list` ability stays, and keeps its meaning — may this user see
|
||||
this document's history. It is a gate now rather than the permission for an
|
||||
endpoint of ours: the frontend hides the history menu without it, and the
|
||||
collaboration server reads it to decide whether to ask this backend when the
|
||||
caller's access began, which is where the history it serves starts.
|
||||
- The JWKS of the resource server moved from `/api/{version}/jwks` to
|
||||
`/external_api/{version}/jwks`, alongside the rest of the resource server
|
||||
endpoints. `/api/{version}/jwks` now publishes the public key validating the
|
||||
|
||||
@@ -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_nb_accesses_cache_key(self):
|
||||
"""Generate a unique cache key for each document."""
|
||||
return f"document_{self.id!s}_nb_accesses"
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -145,7 +145,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
|
||||
)
|
||||
@@ -248,7 +247,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
|
||||
)
|
||||
@@ -307,7 +305,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
|
||||
)
|
||||
@@ -359,7 +356,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
|
||||
)
|
||||
@@ -636,8 +632,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