From e1c7f90154052e7e2d767d94c6323f4b05a6da95 Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Fri, 3 Apr 2026 17:20:49 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(backend)=20create=20a=20dedicated=20e?= =?UTF-8?q?ndpoint=20to=20update=20document=20content?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We want a dedicated endpoint to update a document content. Previously, updating the content was made on the update action shared with all other document's properties. When the title is updated, the response contains the content, so a call to the s3 storage is made and we don't want this. Isolating the content update will allow us in the next commit to remove the content from the Document serializer. --- CHANGELOG.md | 4 + src/backend/core/api/permissions.py | 1 + src/backend/core/api/serializers.py | 31 + src/backend/core/api/viewsets.py | 74 ++- src/backend/core/models.py | 1 + .../test_api_documents_content_update.py | 569 ++++++++++++++++++ .../documents/test_api_documents_retrieve.py | 5 + .../documents/test_api_documents_trashbin.py | 1 + ...pi_documents_update_extract_attachments.py | 52 +- .../core/tests/test_models_documents.py | 11 + 10 files changed, 714 insertions(+), 35 deletions(-) create mode 100644 src/backend/core/tests/documents/test_api_documents_content_update.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d89b178bb..b9477c70b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to ## [Unreleased] +### Added + +- ✨(backend) create a dedicated endpoint to update document content + ### Changed - ♻️(backend) rename documents content endpoint in formatted-content diff --git a/src/backend/core/api/permissions.py b/src/backend/core/api/permissions.py index affd9b070..2b36d59db 100644 --- a/src/backend/core/api/permissions.py +++ b/src/backend/core/api/permissions.py @@ -12,6 +12,7 @@ from core.models import DocumentAccess, RoleChoices, get_trashbin_cutoff ACTION_FOR_METHOD_TO_PERMISSION = { "versions_detail": {"DELETE": "versions_destroy", "GET": "versions_retrieve"}, "children": {"GET": "children_list", "POST": "children_create"}, + "content": {"PATCH": "content_patch"}, } diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 0daf1218c..d96144538 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -358,6 +358,37 @@ class DocumentSerializer(ListDocumentSerializer): return super().save(**kwargs) +class DocumentContentSerializer(serializers.Serializer): + """Serializer for updating only the raw content of a document stored in S3.""" + + content = serializers.CharField(required=True) + websocket = serializers.BooleanField(required=False) + + def validate_content(self, value): + """Validate the content field.""" + if not value: + return None + + try: + b64decode(value, validate=True) + except binascii.Error as err: + raise serializers.ValidationError("Invalid base64 content.") from err + + return value + + def update(self, instance, validated_data): + """ + This serializer does not support updates. + """ + raise NotImplementedError("Update is not supported for this serializer.") + + def create(self, validated_data): + """ + This serializer does not support create. + """ + raise NotImplementedError("Create is not supported for this serializer.") + + class DocumentAccessSerializer(serializers.ModelSerializer): """Serialize document accesses.""" diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 361118abb..2cdb857cf 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -776,17 +776,15 @@ class DocumentViewSet( def perform_update(self, serializer): """Check rules about collaboration.""" if ( - serializer.validated_data.get("websocket", False) - or not settings.COLLABORATION_WS_NOT_CONNECTED_READY_ONLY + not serializer.validated_data.get("websocket", False) + and settings.COLLABORATION_WS_NOT_CONNECTED_READY_ONLY + and not self._can_user_edit_document(serializer.instance.id, set_cache=True) ): - return super().perform_update(serializer) + raise drf.exceptions.PermissionDenied( + "You are not allowed to edit this document." + ) - if self._can_user_edit_document(serializer.instance.id, set_cache=True): - return super().perform_update(serializer) - - raise drf.exceptions.PermissionDenied( - "You are not allowed to edit this document." - ) + return super().perform_update(serializer) @drf.decorators.action( detail=True, @@ -1875,6 +1873,64 @@ class DocumentViewSet( return drf.response.Response("authorized", headers=request.headers, status=200) + @drf.decorators.action(detail=True, methods=["patch"], url_path="content") + def content(self, request, *args, **kwargs): + """Update the raw Yjs content of a document stored in S3.""" + document = self.get_object() + serializer = serializers.DocumentContentSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + + if ( + not serializer.validated_data.get("websocket", False) + and settings.COLLABORATION_WS_NOT_CONNECTED_READY_ONLY + and not self._can_user_edit_document(document.id, set_cache=True) + ): + raise drf.exceptions.PermissionDenied( + "You are not allowed to edit this document." + ) + + content = serializer.validated_data["content"] + + extracted_attachments = set(extract_attachments(content)) + + existing_attachments = set(document.attachments or []) + new_attachments = extracted_attachments - existing_attachments + if new_attachments: + attachments_documents = ( + models.Document.objects.filter( + attachments__overlap=list(new_attachments) + ) + .only("path", "attachments") + .order_by("path") + ) + + user = self.request.user + readable_per_se_paths = ( + models.Document.objects.readable_per_se(user) + .order_by("path") + .values_list("path", flat=True) + ) + readable_attachments_paths = filter_descendants( + [doc.path for doc in attachments_documents], + readable_per_se_paths, + skip_sorting=True, + ) + + readable_attachments = set() + for attachments_document in attachments_documents: + if attachments_document.path not in readable_attachments_paths: + continue + readable_attachments.update( + set(attachments_document.attachments) & new_attachments + ) + + # Update attachments with readable keys + document.attachments = list(existing_attachments | readable_attachments) + document.content = content + document.save() + + return drf_response.Response(status=status.HTTP_204_NO_CONTENT) + @drf.decorators.action(detail=True, methods=["get"], url_path="media-check") def media_check(self, request, *args, **kwargs): """ diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 45a7b0e72..3217f8599 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -1309,6 +1309,7 @@ class Document(MP_Node, BaseModel): "collaboration_auth": can_get, "comment": can_comment, "formatted_content": can_get, + "content_patch": can_update, "cors_proxy": can_get, "descendants": can_get, "destroy": can_destroy, diff --git a/src/backend/core/tests/documents/test_api_documents_content_update.py b/src/backend/core/tests/documents/test_api_documents_content_update.py new file mode 100644 index 000000000..44755b783 --- /dev/null +++ b/src/backend/core/tests/documents/test_api_documents_content_update.py @@ -0,0 +1,569 @@ +""" +Tests for the PATCH /api/v1.0/documents/{id}/content/ endpoint. +""" + +import base64 +from functools import cache +from uuid import uuid4 + +from django.core.cache import cache as django_cache +from django.core.files.storage import default_storage + +import pycrdt +import pytest +import responses +from rest_framework import status +from rest_framework.test import APIClient + +from core import factories, models +from core.tests.conftest import TEAM, USER, VIA + +pytestmark = pytest.mark.django_db + + +@cache +def get_sample_ydoc(): + """Return a ydoc from text for testing purposes.""" + ydoc = pycrdt.Doc() + ydoc["document-store"] = pycrdt.Text("Hello") + update = ydoc.get_update() + return base64.b64encode(update).decode("utf-8") + + +def get_s3_content(document): + """Read the raw content currently stored in S3 for the given document.""" + with default_storage.open(document.file_key, mode="rb") as file: + return file.read().decode() + + +def test_api_documents_content_update_anonymous(): + """Anonymous users without access cannot update document content.""" + document = factories.DocumentFactory(link_reach="restricted") + + response = APIClient().patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_sample_ydoc()}, + ) + + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + +def test_api_documents_content_update_authenticated_no_access(): + """Authenticated users without access cannot update document content.""" + user = factories.UserFactory() + document = factories.DocumentFactory(link_reach="restricted") + + client = APIClient() + client.force_login(user) + + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_sample_ydoc()}, + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.parametrize("role", ["reader", "commenter"]) +def test_api_documents_content_update_read_only_role(role): + """Users with reader or commenter role cannot update document content.""" + user = factories.UserFactory() + document = factories.DocumentFactory(link_reach="restricted") + factories.UserDocumentAccessFactory(document=document, user=user, role=role) + + client = APIClient() + client.force_login(user) + + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_sample_ydoc()}, + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.parametrize("via", VIA) +@pytest.mark.parametrize("role", ["editor", "administrator", "owner"]) +def test_api_documents_content_update_success(role, via, mock_user_teams): + """Users with editor, administrator, or owner role can update document content.""" + user = factories.UserFactory() + document = factories.DocumentFactory(link_reach="restricted") + + if via == USER: + factories.UserDocumentAccessFactory(document=document, user=user, role=role) + elif via == TEAM: + mock_user_teams.return_value = ["lasuite"] + factories.TeamDocumentAccessFactory( + document=document, team="lasuite", role=role + ) + + client = APIClient() + client.force_login(user) + + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_sample_ydoc(), "websocket": True}, + ) + + assert response.status_code == status.HTTP_204_NO_CONTENT + assert get_s3_content(document) == get_sample_ydoc() + + +def test_api_documents_content_update_missing_content_field(): + """A request body without the content field returns 400.""" + user = factories.UserFactory() + document = factories.DocumentFactory(link_reach="restricted") + factories.UserDocumentAccessFactory(document=document, user=user, role="editor") + + client = APIClient() + client.force_login(user) + + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {}, + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json() == { + "content": [ + "This field is required.", + ] + } + + +def test_api_documents_content_update_invalid_base64(): + """A non-base64 content value returns 400.""" + user = factories.UserFactory() + document = factories.DocumentFactory(link_reach="restricted") + factories.UserDocumentAccessFactory(document=document, user=user, role="editor") + + client = APIClient() + client.force_login(user) + + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": "not-valid-base64!!!"}, + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json() == { + "content": [ + "Invalid base64 content.", + ] + } + + +def test_api_documents_content_update_nonexistent_document(): + """Updating the content of a non-existent document returns 404.""" + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + response = client.patch( + f"/api/v1.0/documents/{uuid4()!s}/content/", + {"content": get_sample_ydoc()}, + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND + + +def test_api_documents_content_update_replaces_existing(): + """Patching content replaces whatever was previously in S3.""" + user = factories.UserFactory() + document = factories.DocumentFactory(link_reach="restricted") + factories.UserDocumentAccessFactory(document=document, user=user, role="editor") + + client = APIClient() + client.force_login(user) + + assert get_s3_content(document) == factories.YDOC_HELLO_WORLD_BASE64 + + new_content = get_sample_ydoc() + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": new_content, "websocket": True}, + ) + + assert response.status_code == status.HTTP_204_NO_CONTENT + assert get_s3_content(document) == new_content + + +@pytest.mark.parametrize("role", ["editor", "administrator"]) +def test_api_documents_content_update_deleted_document_for_non_owners(role): + """Updating content on a soft-deleted document returns 404 for non-owners. + + Soft-deleted documents are excluded from the queryset for non-owners, + so the endpoint returns 404 rather than 403. + """ + user = factories.UserFactory() + document = factories.DocumentFactory(link_reach="restricted") + factories.UserDocumentAccessFactory(document=document, user=user, role=role) + + document.soft_delete() + document.refresh_from_db() + + client = APIClient() + client.force_login(user) + + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_sample_ydoc()}, + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND + + +def test_api_documents_content_update_deleted_document_for_owners(): + """Updating content on a soft-deleted document returns 404 for non-owners. + + Soft-deleted documents are excluded from the queryset for non-owners, + so the endpoint returns 404 rather than 403. + """ + user = factories.UserFactory() + document = factories.DocumentFactory(link_reach="restricted") + factories.UserDocumentAccessFactory(document=document, user=user, role="owner") + + document.soft_delete() + document.refresh_from_db() + + client = APIClient() + client.force_login(user) + + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_sample_ydoc()}, + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + + +def test_api_documents_content_update_link_editor(): + """ + A public document with link_role=editor allows any authenticated user to + update content via the link role. + """ + user = factories.UserFactory() + document = factories.DocumentFactory(link_reach="public", link_role="editor") + + client = APIClient() + client.force_login(user) + + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_sample_ydoc(), "websocket": True}, + ) + + assert response.status_code == status.HTTP_204_NO_CONTENT + assert get_s3_content(document) == get_sample_ydoc() + assert models.Document.objects.filter(id=document.id).exists() + + +@responses.activate +def test_api_documents_content_update_authenticated_no_websocket(settings): + """ + When a user updates the document content, not connected to the websocket and is the first + to update, the content should be updated. + """ + user = factories.UserFactory(with_owned_document=True) + client = APIClient() + client.force_login(user) + session_key = client.session.session_key + + document = factories.DocumentFactory(users=[(user, "editor")]) + + settings.COLLABORATION_API_URL = "http://example.com/" + settings.COLLABORATION_SERVER_SECRET = "secret-token" + settings.COLLABORATION_WS_NOT_CONNECTED_READY_ONLY = True + endpoint_url = ( + f"{settings.COLLABORATION_API_URL}get-connections/" + f"?room={document.id}&sessionKey={session_key}" + ) + ws_resp = responses.get(endpoint_url, json={"count": 0, "exists": False}) + + assert django_cache.get(f"docs:no-websocket:{document.id}") is None + + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_sample_ydoc(), "websocket": False}, + ) + assert response.status_code == status.HTTP_204_NO_CONTENT + assert get_s3_content(document) == get_sample_ydoc() + assert django_cache.get(f"docs:no-websocket:{document.id}") == session_key + assert ws_resp.call_count == 1 + + +@responses.activate +def test_api_documents_content_update_authenticated_no_websocket_user_already_editing( + settings, +): + """ + When a user updates the document content, not connected to the websocket and another session + is already editing, the update should be denied. + """ + user = factories.UserFactory(with_owned_document=True) + client = APIClient() + client.force_login(user) + session_key = client.session.session_key + + document = factories.DocumentFactory(users=[(user, "editor")]) + + settings.COLLABORATION_API_URL = "http://example.com/" + settings.COLLABORATION_SERVER_SECRET = "secret-token" + settings.COLLABORATION_WS_NOT_CONNECTED_READY_ONLY = True + endpoint_url = ( + f"{settings.COLLABORATION_API_URL}get-connections/" + f"?room={document.id}&sessionKey={session_key}" + ) + ws_resp = responses.get(endpoint_url, json={"count": 0, "exists": False}) + + django_cache.set(f"docs:no-websocket:{document.id}", "other_session_key") + + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_sample_ydoc(), "websocket": False}, + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + assert response.json() == {"detail": "You are not allowed to edit this document."} + assert ws_resp.call_count == 1 + + +@responses.activate +def test_api_documents_content_update_no_websocket_other_user_connected_to_websocket( + settings, +): + """ + When a user updates document content without websocket and another user is connected + to the websocket, the update should be denied. + """ + user = factories.UserFactory(with_owned_document=True) + client = APIClient() + client.force_login(user) + session_key = client.session.session_key + + document = factories.DocumentFactory(users=[(user, "editor")]) + + settings.COLLABORATION_API_URL = "http://example.com/" + settings.COLLABORATION_SERVER_SECRET = "secret-token" + settings.COLLABORATION_WS_NOT_CONNECTED_READY_ONLY = True + endpoint_url = ( + f"{settings.COLLABORATION_API_URL}get-connections/" + f"?room={document.id}&sessionKey={session_key}" + ) + ws_resp = responses.get(endpoint_url, json={"count": 3, "exists": False}) + + assert django_cache.get(f"docs:no-websocket:{document.id}") is None + + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_sample_ydoc(), "websocket": False}, + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + assert response.json() == {"detail": "You are not allowed to edit this document."} + assert django_cache.get(f"docs:no-websocket:{document.id}") is None + assert ws_resp.call_count == 1 + + +@responses.activate +def test_api_documents_content_update_user_connected_to_websocket(settings): + """ + When a user updates document content and is connected to the websocket, + the content should be updated. + """ + user = factories.UserFactory(with_owned_document=True) + client = APIClient() + client.force_login(user) + session_key = client.session.session_key + + document = factories.DocumentFactory(users=[(user, "editor")]) + + settings.COLLABORATION_API_URL = "http://example.com/" + settings.COLLABORATION_SERVER_SECRET = "secret-token" + settings.COLLABORATION_WS_NOT_CONNECTED_READY_ONLY = True + endpoint_url = ( + f"{settings.COLLABORATION_API_URL}get-connections/" + f"?room={document.id}&sessionKey={session_key}" + ) + ws_resp = responses.get(endpoint_url, json={"count": 3, "exists": True}) + + assert django_cache.get(f"docs:no-websocket:{document.id}") is None + + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_sample_ydoc(), "websocket": False}, + ) + assert response.status_code == status.HTTP_204_NO_CONTENT + assert get_s3_content(document) == get_sample_ydoc() + assert django_cache.get(f"docs:no-websocket:{document.id}") is None + assert ws_resp.call_count == 1 + + +@responses.activate +def test_api_documents_content_update_websocket_server_unreachable_fallback_to_no_websocket( + settings, +): + """ + When the websocket server is unreachable, the content should be updated like if the user + was not connected to the websocket. + """ + user = factories.UserFactory(with_owned_document=True) + client = APIClient() + client.force_login(user) + session_key = client.session.session_key + + document = factories.DocumentFactory(users=[(user, "editor")]) + + settings.COLLABORATION_API_URL = "http://example.com/" + settings.COLLABORATION_SERVER_SECRET = "secret-token" + settings.COLLABORATION_WS_NOT_CONNECTED_READY_ONLY = True + endpoint_url = ( + f"{settings.COLLABORATION_API_URL}get-connections/" + f"?room={document.id}&sessionKey={session_key}" + ) + ws_resp = responses.get(endpoint_url, status=500) + + assert django_cache.get(f"docs:no-websocket:{document.id}") is None + + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_sample_ydoc(), "websocket": False}, + ) + assert response.status_code == status.HTTP_204_NO_CONTENT + assert get_s3_content(document) == get_sample_ydoc() + assert django_cache.get(f"docs:no-websocket:{document.id}") == session_key + assert ws_resp.call_count == 1 + + +@responses.activate +def test_api_content_update_websocket_server_unreachable_fallback_to_no_websocket_other_users( + settings, +): + """ + When the websocket server is unreachable, the behavior fallback to the no websocket one. + If another user is already editing, the content update should be denied. + """ + user = factories.UserFactory(with_owned_document=True) + client = APIClient() + client.force_login(user) + session_key = client.session.session_key + + document = factories.DocumentFactory(users=[(user, "editor")]) + + settings.COLLABORATION_API_URL = "http://example.com/" + settings.COLLABORATION_SERVER_SECRET = "secret-token" + settings.COLLABORATION_WS_NOT_CONNECTED_READY_ONLY = True + endpoint_url = ( + f"{settings.COLLABORATION_API_URL}get-connections/" + f"?room={document.id}&sessionKey={session_key}" + ) + ws_resp = responses.get(endpoint_url, status=500) + + django_cache.set(f"docs:no-websocket:{document.id}", "other_session_key") + + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_sample_ydoc(), "websocket": False}, + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + assert django_cache.get(f"docs:no-websocket:{document.id}") == "other_session_key" + assert ws_resp.call_count == 1 + + +@responses.activate +def test_api_content_update_websocket_server_room_not_found_fallback_to_no_websocket_other_users( + settings, +): + """ + When the WebSocket server does not have the room created, the logic should fallback to + no-WebSocket. If another user is already editing, the update must be denied. + """ + user = factories.UserFactory(with_owned_document=True) + client = APIClient() + client.force_login(user) + session_key = client.session.session_key + + document = factories.DocumentFactory(users=[(user, "editor")]) + + settings.COLLABORATION_API_URL = "http://example.com/" + settings.COLLABORATION_SERVER_SECRET = "secret-token" + settings.COLLABORATION_WS_NOT_CONNECTED_READY_ONLY = True + endpoint_url = ( + f"{settings.COLLABORATION_API_URL}get-connections/" + f"?room={document.id}&sessionKey={session_key}" + ) + ws_resp = responses.get(endpoint_url, status=404) + + django_cache.set(f"docs:no-websocket:{document.id}", "other_session_key") + + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_sample_ydoc(), "websocket": False}, + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + assert django_cache.get(f"docs:no-websocket:{document.id}") == "other_session_key" + assert ws_resp.call_count == 1 + + +@responses.activate +def test_api_documents_content_update_force_websocket_param_to_true(settings): + """ + When the websocket parameter is set to true, the content should be updated without any check. + """ + user = factories.UserFactory(with_owned_document=True) + client = APIClient() + client.force_login(user) + session_key = client.session.session_key + + document = factories.DocumentFactory(users=[(user, "editor")]) + + settings.COLLABORATION_API_URL = "http://example.com/" + settings.COLLABORATION_SERVER_SECRET = "secret-token" + settings.COLLABORATION_WS_NOT_CONNECTED_READY_ONLY = True + endpoint_url = ( + f"{settings.COLLABORATION_API_URL}get-connections/" + f"?room={document.id}&sessionKey={session_key}" + ) + ws_resp = responses.get(endpoint_url, status=500) + + assert django_cache.get(f"docs:no-websocket:{document.id}") is None + + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_sample_ydoc(), "websocket": True}, + ) + assert response.status_code == status.HTTP_204_NO_CONTENT + assert get_s3_content(document) == get_sample_ydoc() + assert django_cache.get(f"docs:no-websocket:{document.id}") is None + assert ws_resp.call_count == 0 + + +@responses.activate +def test_api_documents_content_update_feature_flag_disabled(settings): + """ + When the feature flag is disabled, the content should be updated without any check. + """ + user = factories.UserFactory(with_owned_document=True) + client = APIClient() + client.force_login(user) + session_key = client.session.session_key + + document = factories.DocumentFactory(users=[(user, "editor")]) + + settings.COLLABORATION_API_URL = "http://example.com/" + settings.COLLABORATION_SERVER_SECRET = "secret-token" + settings.COLLABORATION_WS_NOT_CONNECTED_READY_ONLY = False + endpoint_url = ( + f"{settings.COLLABORATION_API_URL}get-connections/" + f"?room={document.id}&sessionKey={session_key}" + ) + ws_resp = responses.get(endpoint_url, status=500) + + assert django_cache.get(f"docs:no-websocket:{document.id}") is None + + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_sample_ydoc(), "websocket": False}, + ) + assert response.status_code == status.HTTP_204_NO_CONTENT + assert get_s3_content(document) == get_sample_ydoc() + assert django_cache.get(f"docs:no-websocket:{document.id}") is None + assert ws_resp.call_count == 0 diff --git a/src/backend/core/tests/documents/test_api_documents_retrieve.py b/src/backend/core/tests/documents/test_api_documents_retrieve.py index e9d05c577..48c6911ff 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -53,6 +53,7 @@ def test_api_documents_retrieve_anonymous_public_standalone(): "restricted": None, }, "mask": False, + "content_patch": document.link_role == "editor", "media_auth": True, "media_check": True, "move": False, @@ -131,6 +132,7 @@ def test_api_documents_retrieve_anonymous_public_parent(): **links_definition ), "mask": False, + "content_patch": grand_parent.link_role == "editor", "media_auth": True, "media_check": True, "move": False, @@ -242,6 +244,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated( "restricted": None, }, "mask": True, + "content_patch": document.link_role == "editor", "media_auth": True, "media_check": True, "move": False, @@ -328,6 +331,7 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea ), "mask": True, "move": False, + "content_patch": grand_parent.link_role == "editor", "media_auth": True, "media_check": True, "partial_update": grand_parent.link_role == "editor", @@ -527,6 +531,7 @@ def test_api_documents_retrieve_authenticated_related_parent(): **link_definition ), "mask": True, + "content_patch": access.role not in ["reader", "commenter"], "media_auth": True, "media_check": True, "move": access.role in ["administrator", "owner"], diff --git a/src/backend/core/tests/documents/test_api_documents_trashbin.py b/src/backend/core/tests/documents/test_api_documents_trashbin.py index 16a1bd2d5..4efb1ef20 100644 --- a/src/backend/core/tests/documents/test_api_documents_trashbin.py +++ b/src/backend/core/tests/documents/test_api_documents_trashbin.py @@ -95,6 +95,7 @@ def test_api_documents_trashbin_format(): "restricted": None, }, "mask": False, + "content_patch": False, "media_auth": False, "media_check": False, "move": False, # Can't move a deleted document diff --git a/src/backend/core/tests/documents/test_api_documents_update_extract_attachments.py b/src/backend/core/tests/documents/test_api_documents_update_extract_attachments.py index 5623749fd..a9d04c4ac 100644 --- a/src/backend/core/tests/documents/test_api_documents_update_extract_attachments.py +++ b/src/backend/core/tests/documents/test_api_documents_update_extract_attachments.py @@ -14,7 +14,7 @@ from core import factories pytestmark = pytest.mark.django_db -def get_ydoc_with_mages(image_keys): +def get_ydoc_with_images(image_keys): """Return a ydoc from text for testing purposes.""" ydoc = pycrdt.Doc() fragment = pycrdt.XmlFragment( @@ -36,7 +36,7 @@ def test_api_documents_update_new_attachment_keys_anonymous(django_assert_num_qu """ image_keys = [f"{uuid4()!s}/attachments/{uuid4()!s}.png" for _ in range(4)] document = factories.DocumentFactory( - content=get_ydoc_with_mages(image_keys[:1]), + content=get_ydoc_with_images(image_keys[:1]), attachments=[image_keys[0]], link_reach="public", link_role="editor", @@ -47,13 +47,13 @@ def test_api_documents_update_new_attachment_keys_anonymous(django_assert_num_qu factories.DocumentFactory(attachments=[image_keys[3]], link_reach="restricted") expected_keys = {image_keys[i] for i in [0, 1]} - with django_assert_num_queries(11): - response = APIClient().put( - f"/api/v1.0/documents/{document.id!s}/", - {"content": get_ydoc_with_mages(image_keys), "websocket": True}, + with django_assert_num_queries(9): + response = APIClient().patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_ydoc_with_images(image_keys)}, format="json", ) - assert response.status_code == 200 + assert response.status_code == 204 document.refresh_from_db() assert set(document.attachments) == expected_keys @@ -61,12 +61,12 @@ def test_api_documents_update_new_attachment_keys_anonymous(django_assert_num_qu # Check that the db query to check attachments readability for extracted # keys is not done if the content changes but no new keys are found with django_assert_num_queries(7): - response = APIClient().put( - f"/api/v1.0/documents/{document.id!s}/", - {"content": get_ydoc_with_mages(image_keys[:2]), "websocket": True}, + response = APIClient().patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_ydoc_with_images(image_keys[:2]), "websocket": True}, format="json", ) - assert response.status_code == 200 + assert response.status_code == 204 document.refresh_from_db() assert len(document.attachments) == 2 @@ -87,7 +87,7 @@ def test_api_documents_update_new_attachment_keys_authenticated( image_keys = [f"{uuid4()!s}/attachments/{uuid4()!s}.png" for _ in range(5)] document = factories.DocumentFactory( - content=get_ydoc_with_mages(image_keys[:1]), + content=get_ydoc_with_images(image_keys[:1]), attachments=[image_keys[0]], users=[(user, "editor")], ) @@ -98,13 +98,13 @@ def test_api_documents_update_new_attachment_keys_authenticated( factories.DocumentFactory(attachments=[image_keys[4]], users=[user]) expected_keys = {image_keys[i] for i in [0, 1, 2, 4]} - with django_assert_num_queries(12): - response = client.put( - f"/api/v1.0/documents/{document.id!s}/", - {"content": get_ydoc_with_mages(image_keys)}, + with django_assert_num_queries(10): + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_ydoc_with_images(image_keys)}, format="json", ) - assert response.status_code == 200 + assert response.status_code == 204 document.refresh_from_db() assert set(document.attachments) == expected_keys @@ -112,12 +112,12 @@ def test_api_documents_update_new_attachment_keys_authenticated( # Check that the db query to check attachments readability for extracted # keys is not done if the content changes but no new keys are found with django_assert_num_queries(8): - response = client.put( - f"/api/v1.0/documents/{document.id!s}/", - {"content": get_ydoc_with_mages(image_keys[:2])}, + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_ydoc_with_images(image_keys[:2])}, format="json", ) - assert response.status_code == 200 + assert response.status_code == 204 document.refresh_from_db() assert len(document.attachments) == 4 @@ -135,19 +135,19 @@ def test_api_documents_update_new_attachment_keys_duplicate(): image_key1 = f"{uuid4()!s}/attachments/{uuid4()!s}.png" image_key2 = f"{uuid4()!s}/attachments/{uuid4()!s}.png" document = factories.DocumentFactory( - content=get_ydoc_with_mages([image_key1]), + content=get_ydoc_with_images([image_key1]), attachments=[image_key1], users=[(user, "editor")], ) factories.DocumentFactory(attachments=[image_key2], users=[user]) - response = client.put( - f"/api/v1.0/documents/{document.id!s}/", - {"content": get_ydoc_with_mages([image_key1, image_key2, image_key2])}, + response = client.patch( + f"/api/v1.0/documents/{document.id!s}/content/", + {"content": get_ydoc_with_images([image_key1, image_key2, image_key2])}, format="json", ) - assert response.status_code == 200 + assert response.status_code == 204 document.refresh_from_db() assert len(document.attachments) == 2 diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index 434aef1a0..8a25e13bd 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -172,6 +172,7 @@ def test_models_documents_get_abilities_forbidden( "comment": False, "invite_owner": False, "mask": False, + "content_patch": False, "media_auth": False, "media_check": False, "move": False, @@ -245,6 +246,7 @@ def test_models_documents_get_abilities_reader( "restricted": None, }, "mask": is_authenticated, + "content_patch": False, "media_auth": True, "media_check": True, "move": False, @@ -317,6 +319,7 @@ def test_models_documents_get_abilities_commenter( "restricted": None, }, "mask": is_authenticated, + "content_patch": False, "media_auth": True, "media_check": True, "move": False, @@ -386,6 +389,7 @@ def test_models_documents_get_abilities_editor( "restricted": None, }, "mask": is_authenticated, + "content_patch": True, "media_auth": True, "media_check": True, "move": False, @@ -444,6 +448,7 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): "restricted": None, }, "mask": True, + "content_patch": True, "media_auth": True, "media_check": True, "move": True, @@ -488,6 +493,7 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): "restricted": None, }, "mask": False, + "content_patch": False, "media_auth": False, "media_check": False, "move": False, @@ -536,6 +542,7 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries) "restricted": None, }, "mask": True, + "content_patch": True, "media_auth": True, "media_check": True, "move": True, @@ -594,6 +601,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): "restricted": None, }, "mask": True, + "content_patch": True, "media_auth": True, "media_check": True, "move": False, @@ -660,6 +668,7 @@ def test_models_documents_get_abilities_reader_user( "restricted": None, }, "mask": True, + "content_patch": access_from_link, "media_auth": True, "media_check": True, "move": False, @@ -727,6 +736,7 @@ def test_models_documents_get_abilities_commenter_user( "restricted": None, }, "mask": True, + "content_patch": access_from_link, "media_auth": True, "media_check": True, "move": False, @@ -790,6 +800,7 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries): "restricted": None, }, "mask": True, + "content_patch": False, "media_auth": True, "media_check": True, "move": False,