diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d5accb78..d8e0e9c15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -140,6 +140,7 @@ and this project adheres to pending). The get-connections API is dropped for good: its only consumer was the removed can-edit mechanism, so it is not needed anymore - 🔥(backend) remove the unused `CollaborationService` +- 💥(backend) remove the `documents/{id}/content/` endpoint - 💥(backend) remove the `documents/{id}/can-edit/` endpoint - 💥(y-provider) the published `lasuite/impress-y-provider` image becomes converter-only and no longer serves `/collaboration/ws/`; deployments using diff --git a/UPGRADE.md b/UPGRADE.md index feafc80ff..06629eb5d 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -16,6 +16,15 @@ the following command inside your docker container: ## [Unreleased] +- The endpoint `/api/v1.0/documents/{document_id}/content/`, added in 5.0.0, is + removed, both its `GET` and its `PATCH`. The content of a document is now + saved and served by the collaboration server, the editor exchanging it over + the websocket, so nothing reads or writes it through the API anymore. If you + integrate with Docs, stop calling this endpoint: the `content_patch` and + `content_retrieve` abilities disappear from the document payload along with + 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 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 diff --git a/src/backend/core/api/permissions.py b/src/backend/core/api/permissions.py index 4b92b711e..affd9b070 100644 --- a/src/backend/core/api/permissions.py +++ b/src/backend/core/api/permissions.py @@ -12,7 +12,6 @@ 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", "GET": "content_retrieve"}, } diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 5bf57ebd8..eb8f27261 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -1,9 +1,7 @@ """Client serializers for the impress core app.""" # pylint: disable=too-many-lines -import binascii import mimetypes -from base64 import b64decode from os.path import splitext from django.conf import settings @@ -308,33 +306,6 @@ class SearchDocumentSerializer(ListDocumentSerializer): read_only_fields = ListDocumentSerializer.Meta.read_only_fields + ["parent"] -class DocumentContentSerializer(serializers.Serializer): - """Serializer for updating only the raw content of a document stored in S3.""" - - content = serializers.CharField(required=True) - - def validate_content(self, value): - """Validate the content field.""" - 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/utils.py b/src/backend/core/api/utils.py index b44f486cf..5a6d00f90 100644 --- a/src/backend/core/api/utils.py +++ b/src/backend/core/api/utils.py @@ -1,6 +1,5 @@ """Util to generate S3 authorization headers for object storage access control""" -import datetime as dt import time from abc import ABC, abstractmethod @@ -197,36 +196,3 @@ class AIUserRateThrottle(AIBaseRateThrottle): if x_forwarded_for else request.META.get("REMOTE_ADDR") ) - - -def get_content_metadata_cache_key(document_id): - """Return the cache key used to store content metadata.""" - return f"docs:content-metadata:{document_id!s}" - - -def parse_http_conditional_headers(request): - """Extract and normalize `If-None-Match` and `If-Modified-Since`. - - The `W/` weak prefix is stripped from the ETag because reverse proxies - (e.g. nginx with gzip) rewrite strong ETags into weak ones, which would - otherwise break a strict equality check in production. - """ - if_none_match = request.META.get("HTTP_IF_NONE_MATCH") - if if_none_match and if_none_match.startswith("W/"): - if_none_match = if_none_match.removeprefix("W/") - - if_modified_since_dt = None - if not (if_modified_since := request.META.get("HTTP_IF_MODIFIED_SINCE")): - return if_none_match, if_modified_since_dt - - try: - if_modified_since_dt = dt.datetime.strptime( - if_modified_since, "%a, %d %b %Y %H:%M:%S %Z" - ) - except ValueError: - if_modified_since_dt = None - else: - if not if_modified_since_dt.tzinfo: - if_modified_since_dt = if_modified_since_dt.replace(tzinfo=dt.timezone.utc) - - return if_none_match, if_modified_since_dt diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 8cd5b7f32..fa9424157 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -3,14 +3,12 @@ # pylint: disable=too-many-lines import base64 -import datetime as dt import ipaddress import json import logging import socket import uuid from collections import defaultdict -from io import BytesIO from urllib.parse import unquote, urlencode, urlparse from django.conf import settings @@ -20,7 +18,7 @@ from django.core.cache import cache from django.core.exceptions import ValidationError from django.core.files.storage import default_storage from django.core.validators import URLValidator -from django.db import DatabaseError, connection, transaction +from django.db import DatabaseError, transaction from django.db import models as db from django.db.models.expressions import RawSQL from django.db.models.functions import Greatest, Left, Length @@ -36,7 +34,6 @@ import requests import rest_framework as drf import waffle from botocore.exceptions import ClientError -from botocore.response import StreamingBody from csp.constants import NONE from csp.decorators import csp_update from lasuite.malware_detection import malware_detection @@ -78,10 +75,9 @@ from core.utils.analytics import PosthogEventName, posthog_capture from core.utils.dicts import lowercase_keys from core.utils.paths import filter_descendants from core.utils.s3 import get_s3_client -from core.utils.s3_response_stream import content_stream from core.utils.treebeard import create_tree_node_with_retry from core.utils.users import users_sharing_documents_with -from core.utils.yjs import extract_attachments, extract_attachments_from_update +from core.utils.yjs import extract_attachments_from_update from ..enums import FeatureFlag, SearchType from . import permissions, serializers, utils @@ -2074,156 +2070,6 @@ class DocumentViewSet( return drf.response.Response("authorized", headers=request.headers, status=200) - @drf.decorators.action(detail=True, methods=["patch"]) - 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) - - content = serializer.validated_data["content"] - try: - extracted_attachments = set(extract_attachments(content)) - except ValueError: - return drf_response.Response( - "invalid yjs document", status=status.HTTP_400_BAD_REQUEST - ) - - existing_attachments = set(document.attachments or []) - new_attachments = extracted_attachments - existing_attachments - - # Ensure we update attachments the request user is allowed to read - 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() - cache.delete(utils.get_content_metadata_cache_key(document.id)) - - return drf_response.Response(status=status.HTTP_204_NO_CONTENT) - - @content.mapping.get - def content_retrieve(self, request, *args, **kwargs): - """ - Retrieve the raw content file from s3 and stream it. - - We implement a HTTP cache based on the ETag and LastModified headers. - The ETag and LastModified are retrieved in the S3 get_object operation to be consistent with - the content Body retrieved at the same time. These metadata are saved in cache for - future requests. - We check in the request if the ETag is present in the If-None-Match header and if it's the - same as the one from the S3 get_object, we return a 304 response. - If the ETag is not present or not the same, we do the same check based on the LastModified - value if present in the If-Modified-Since header. - """ - document = self.get_object() - # The S3 call to fetch the document can take time and the database - # connection is useless in this process. Hence we are closing it now - # to prevent having a massive number of database connections during - # the web-socket re-connection burst. - connection.close() - - if_none_match, if_modified_since_dt = utils.parse_http_conditional_headers( - request - ) - - # First check if a cache is existing to return earlier a 304 without reaching s3 - # if etag or last_modified have not changed. - cache_key = utils.get_content_metadata_cache_key(document.id) - if content_metadata := cache.get(cache_key): - if (if_none_match and if_none_match == content_metadata.get("etag")) or ( - if_modified_since_dt - and dt.datetime.fromisoformat(content_metadata.get("last_modified")) - <= if_modified_since_dt - ): - return drf_response.Response(status=status.HTTP_304_NOT_MODIFIED) - - # Prepare get_object S3 operation. The get_object manages ETag and last_modified - # headers will raise a 304 client error if one of them matches the value existing in - # S3. - get_object_kwargs = { - "Bucket": default_storage.bucket_name, - "Key": document.file_key, - } - if if_none_match: - get_object_kwargs["IfNoneMatch"] = if_none_match - if if_modified_since_dt: - get_object_kwargs["IfModifiedSince"] = if_modified_since_dt - - try: - s3_response = default_storage.connection.meta.client.get_object( - **get_object_kwargs - ) - except ClientError as exc: - code = exc.response["Error"]["Code"] - match code: - case "304" | "PreconditionFailed" | "NotModified": - return drf_response.Response(status=status.HTTP_304_NOT_MODIFIED) - case "NoSuchKey" | "404": - return StreamingHttpResponse( - content_stream(StreamingBody(BytesIO(b""), content_length=0)), - content_type="text/plain", - status=200, - ) - case _: - raise - - last_modified = s3_response["LastModified"] - etag = s3_response["ETag"] - size = s3_response["ContentLength"] - - # Refresh the metadata cache - cache.set( - cache_key, - { - "last_modified": last_modified.isoformat(), - "etag": etag, - }, - settings.CONTENT_METADATA_CACHE_TIMEOUT, - ) - - response = StreamingHttpResponse( - streaming_content=content_stream(s3_response["Body"]), - content_type="text/plain", - status=status.HTTP_200_OK, - ) - - response["Content-Length"] = size - response["ETag"] = etag - response["Last-Modified"] = last_modified.strftime("%a, %d %b %Y %H:%M:%S %Z") - response["Cache-Control"] = "private, no-cache" - - return response - @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 edb148119..3dc0231ce 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -1397,8 +1397,6 @@ class Document(MP_Node, BaseModel): "collaboration_auth": can_get, "comment": can_comment, "formatted_content": can_get, - "content_patch": can_update, - "content_retrieve": retrieve, "cors_proxy": can_get, "descendants": can_get, "destroy": can_destroy, diff --git a/src/backend/core/tests/documents/test_api_documents_content_retrieve.py b/src/backend/core/tests/documents/test_api_documents_content_retrieve.py deleted file mode 100644 index 3d1ce1c3b..000000000 --- a/src/backend/core/tests/documents/test_api_documents_content_retrieve.py +++ /dev/null @@ -1,506 +0,0 @@ -""" -Tests for the GET /api/v1.0/documents/{id}/content/ endpoint. -""" - -from datetime import timedelta -from uuid import uuid4 - -from django.core.cache import cache -from django.core.files.storage import default_storage -from django.utils import timezone - -import pytest -from asgiref.sync import sync_to_async -from rest_framework import status -from rest_framework.test import APIClient - -from core import factories -from core.api.utils import get_content_metadata_cache_key -from core.tests.conftest import TEAM, USER, VIA - -pytestmark = pytest.mark.django_db - - -@pytest.mark.parametrize("reach", ["authenticated", "restricted"]) -def test_api_documents_content_retrieve_anonymous_non_public(reach): - """Anonymous users cannot retrieve content of non-public documents.""" - document = factories.DocumentFactory(link_reach=reach) - - response = APIClient().get(f"/api/v1.0/documents/{document.id!s}/content/") - - assert response.status_code == status.HTTP_401_UNAUTHORIZED - - -def test_api_documents_content_retrieve_anonymous_public(): - """Anonymous users can retrieve content of a public document.""" - document = factories.DocumentFactory(link_reach="public") - - assert not cache.get(get_content_metadata_cache_key(document.id)) - - response = APIClient().get(f"/api/v1.0/documents/{document.id!s}/content/") - - assert response.status_code == status.HTTP_200_OK - assert response["Content-Type"] == "text/plain" - assert b"".join( - response.streaming_content - ) == factories.YDOC_HELLO_WORLD_BASE64.encode("utf-8") - assert response["Content-Length"] is not None - assert response["ETag"] is not None - assert response["Last-Modified"] is not None - assert response["Cache-Control"] == "private, no-cache" - - assert cache.get(get_content_metadata_cache_key(document.id)) - - -def test_api_documents_content_retrieve_authenticated_no_access(): - """Authenticated users without access cannot retrieve content of a restricted document.""" - user = factories.UserFactory() - document = factories.DocumentFactory(link_reach="restricted") - - client = APIClient() - client.force_login(user) - - response = client.get(f"/api/v1.0/documents/{document.id!s}/content/") - - assert response.status_code == status.HTTP_403_FORBIDDEN - - -@pytest.mark.parametrize("link_reach", ["authenticated", "public"]) -def test_api_documents_content_retrieve_authenticated_not_restricted(link_reach): - """ - Authenticated users can retrieve content of a public document - without any explicit access grant. - """ - user = factories.UserFactory() - document = factories.DocumentFactory(link_reach=link_reach) - - client = APIClient() - client.force_login(user) - - assert not cache.get(get_content_metadata_cache_key(document.id)) - - response = client.get(f"/api/v1.0/documents/{document.id!s}/content/") - - assert response.status_code == status.HTTP_200_OK - assert b"".join( - response.streaming_content - ) == factories.YDOC_HELLO_WORLD_BASE64.encode("utf-8") - assert response["Content-Length"] is not None - assert response["ETag"] is not None - assert response["Last-Modified"] is not None - assert response["Cache-Control"] == "private, no-cache" - - assert cache.get(get_content_metadata_cache_key(document.id)) - - -@pytest.mark.parametrize("via", VIA) -@pytest.mark.parametrize( - "role", ["reader", "commenter", "editor", "administrator", "owner"] -) -def test_api_documents_content_retrieve_success(role, via, mock_user_teams): - """Users with any role can retrieve document content, directly or via a team.""" - 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) - - assert not cache.get(get_content_metadata_cache_key(document.id)) - - response = client.get(f"/api/v1.0/documents/{document.id!s}/content/") - - assert response.status_code == status.HTTP_200_OK - assert b"".join( - response.streaming_content - ) == factories.YDOC_HELLO_WORLD_BASE64.encode("utf-8") - assert response["Content-Length"] is not None - assert response["ETag"] is not None - assert response["Last-Modified"] is not None - assert response["Cache-Control"] == "private, no-cache" - - assert cache.get(get_content_metadata_cache_key(document.id)) - - -def test_api_documents_content_retrieve_nonexistent_document(): - """Retrieving content of a non-existent document returns 404.""" - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - response = client.get(f"/api/v1.0/documents/{uuid4()!s}/content/") - - assert response.status_code == status.HTTP_404_NOT_FOUND - - -def test_api_documents_content_retrieve_file_not_in_storage(): - """Returns an empty string when the file does not exist on the storage.""" - user = factories.UserFactory() - document = factories.DocumentFactory(link_reach="restricted") - factories.UserDocumentAccessFactory(document=document, user=user, role="reader") - - client = APIClient() - client.force_login(user) - - default_storage.delete(document.file_key) - - assert not default_storage.exists(document.file_key) - - response = client.get(f"/api/v1.0/documents/{document.id!s}/content/") - - assert response.status_code == status.HTTP_200_OK - assert b"".join(response.streaming_content) == b"" - assert not response.get("Content-Length") - assert not response.get("ETag") - assert not response.get("Last-Modified") - assert not response.get("Cache-Control") - - assert not cache.get(get_content_metadata_cache_key(document.id)) - - -# The data created in this test through `sync_to_async` is written on a -# separate thread-local database connection, outside the atomic transaction -# pytest-django uses to isolate tests. `transaction=True` makes pytest-django -# flush the tables after the test instead of relying on a rollback, so the row -# does not leak into the rest of the suite. -@pytest.mark.django_db(transaction=True) -@pytest.mark.asyncio(loop_scope="function") -async def test_api_documents_content_retrieve_async(monkeypatch): - """ - Test the content retrieve method in async should use the async generator in the streaming - response. - """ - monkeypatch.setenv("PYTHON_SERVER_MODE", "async") - - document = await sync_to_async(factories.DocumentFactory)(link_reach="public") - client = APIClient() - - response = await sync_to_async(client.get)( - f"/api/v1.0/documents/{document.id!s}/content/" - ) - - assert response.status_code == status.HTTP_200_OK - # Wait for the streaming content to be fully received => async iterator -> list - # This fails if the streaming is not an async generator - response_content = b"".join( - [content async for content in response.streaming_content] - ).decode("utf-8") - assert response_content == factories.YDOC_HELLO_WORLD_BASE64 - - -@pytest.mark.django_db(transaction=True) -@pytest.mark.asyncio(loop_scope="function") -async def test_api_documents_content_retrieve_file_not_in_storage_async(monkeypatch): - """Returns an empty string when the file does not exist on the storage.""" - monkeypatch.setenv("PYTHON_SERVER_MODE", "async") - - user = await sync_to_async(factories.UserFactory)() - document = await sync_to_async(factories.DocumentFactory)(link_reach="restricted") - await sync_to_async(factories.UserDocumentAccessFactory)( - document=document, user=user, role="reader" - ) - - client = APIClient() - await client.aforce_login(user) - - await sync_to_async(default_storage.delete)(document.file_key) - - assert not await sync_to_async(default_storage.exists)(document.file_key) - - response = await sync_to_async(client.get)( - f"/api/v1.0/documents/{document.id!s}/content/" - ) - - assert response.status_code == status.HTTP_200_OK - # Wait for the streaming content to be fully received => async iterator -> list - # This fails if the streaming is not an async generator - assert b"".join([content async for content in response.streaming_content]) == b"" - assert not response.get("Content-Length") - assert not response.get("ETag") - assert not response.get("Last-Modified") - assert not response.get("Cache-Control") - - assert not await cache.aget(get_content_metadata_cache_key(document.id)) - - -def test_api_documents_content_retrieve_content_length_header(): - """The response includes the Content-Length header when available from storage.""" - user = factories.UserFactory() - document = factories.DocumentFactory(link_reach="restricted") - factories.UserDocumentAccessFactory(document=document, user=user, role="reader") - - client = APIClient() - client.force_login(user) - - response = client.get(f"/api/v1.0/documents/{document.id!s}/content/") - - assert response.status_code == status.HTTP_200_OK - expected_size = default_storage.size(document.file_key) - assert int(response["Content-Length"]) == expected_size - - -@pytest.mark.parametrize("role", ["reader", "commenter", "editor", "administrator"]) -def test_api_documents_content_retrieve_deleted_document_for_non_owners_all_roles(role): - """ - Retrieving content of a soft-deleted document returns 404 for any non-owner role. - """ - 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.get(f"/api/v1.0/documents/{document.id!s}/content/") - - assert response.status_code == status.HTTP_404_NOT_FOUND - - -def test_api_documents_content_retrieve_deleted_document_for_owner(): - """ - Owners can still retrieve content of a soft-deleted document. - - The 'retrieve' ability is True for owners regardless of deletion state. - """ - 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) - - assert not cache.get(get_content_metadata_cache_key(document.id)) - - response = client.get(f"/api/v1.0/documents/{document.id!s}/content/") - - assert response.status_code == status.HTTP_200_OK - assert b"".join( - response.streaming_content - ) == factories.YDOC_HELLO_WORLD_BASE64.encode("utf-8") - assert response["Content-Length"] is not None - assert response["ETag"] is not None - assert response["Last-Modified"] is not None - assert response["Cache-Control"] == "private, no-cache" - - assert cache.get(get_content_metadata_cache_key(document.id)) - - -def test_api_documents_content_retrieve_reusing_etag(): - """Fetching content reusing a valid ETag header should return a 304.""" - - user = factories.UserFactory() - document = factories.DocumentFactory(link_reach="restricted") - factories.UserDocumentAccessFactory(document=document, user=user, role="owner") - - client = APIClient() - client.force_login(user) - - file_metadata = default_storage.connection.meta.client.head_object( - Bucket=default_storage.bucket_name, Key=document.file_key - ) - last_modified = file_metadata["LastModified"] - etag = file_metadata["ETag"] - size = file_metadata["ContentLength"] - - cache.set( - get_content_metadata_cache_key(document.id), - { - "last_modified": last_modified.isoformat(), - "etag": etag, - "size": size, - }, - ) - - response = client.get( - f"/api/v1.0/documents/{document.id!s}/content/", - headers={"If-None-Match": etag}, - ) - - assert response.status_code == status.HTTP_304_NOT_MODIFIED - - -def test_api_documents_content_retrieve_reusing_invalid_etag(): - """Fetching content using an invalid ETag header should return a 200.""" - - user = factories.UserFactory() - document = factories.DocumentFactory(link_reach="restricted") - factories.UserDocumentAccessFactory(document=document, user=user, role="owner") - - client = APIClient() - client.force_login(user) - - file_metadata = default_storage.connection.meta.client.head_object( - Bucket=default_storage.bucket_name, Key=document.file_key - ) - last_modified = file_metadata["LastModified"] - etag = file_metadata["ETag"] - size = file_metadata["ContentLength"] - - cache.set( - get_content_metadata_cache_key(document.id), - { - "last_modified": last_modified.isoformat(), - "etag": etag, - "size": size, - }, - ) - - response = client.get( - f"/api/v1.0/documents/{document.id!s}/content/", - headers={"If-None-Match": "invalid"}, - ) - - assert response.status_code == status.HTTP_200_OK - assert b"".join( - response.streaming_content - ) == factories.YDOC_HELLO_WORLD_BASE64.encode("utf-8") - assert response["Content-Length"] is not None - assert response["ETag"] is not None - assert response["Last-Modified"] is not None - assert response["Cache-Control"] == "private, no-cache" - - -def test_api_documents_content_retrieve_using_etag_without_cache(): - """ - Fetching content using a valid ETag header but without existing cache should return a 304. - """ - - user = factories.UserFactory() - document = factories.DocumentFactory(link_reach="restricted") - factories.UserDocumentAccessFactory(document=document, user=user, role="owner") - - client = APIClient() - client.force_login(user) - - file_metadata = default_storage.connection.meta.client.head_object( - Bucket=default_storage.bucket_name, Key=document.file_key - ) - etag = file_metadata["ETag"] - - assert not cache.get(get_content_metadata_cache_key(document.id)) - - response = client.get( - f"/api/v1.0/documents/{document.id!s}/content/", - headers={"If-None-Match": etag}, - ) - - assert response.status_code == status.HTTP_304_NOT_MODIFIED - - -def test_api_documents_content_retrieve_reusing_last_modified_since(): - """Fetching a content using a If-Modified-Since valid should return a 304.""" - - user = factories.UserFactory() - document = factories.DocumentFactory(link_reach="restricted") - factories.UserDocumentAccessFactory(document=document, user=user, role="owner") - - client = APIClient() - client.force_login(user) - - file_metadata = default_storage.connection.meta.client.head_object( - Bucket=default_storage.bucket_name, Key=document.file_key - ) - last_modified = file_metadata["LastModified"] - etag = file_metadata["ETag"] - size = file_metadata["ContentLength"] - - cache.set( - get_content_metadata_cache_key(document.id), - { - "last_modified": last_modified.isoformat(), - "etag": etag, - "size": size, - }, - ) - - response = client.get( - f"/api/v1.0/documents/{document.id!s}/content/", - headers={ - "If-Modified-Since": timezone.now().strftime("%a, %d %b %Y %H:%M:%S %Z") - }, - ) - - assert response.status_code == status.HTTP_304_NOT_MODIFIED - - -def test_api_documents_content_retrieve_using_last_modified_since_without_cache(): - """ - Fetching a content using a If-Modified-Since valid should return a 304 - even if content metadata are not present in cache. - """ - - user = factories.UserFactory() - document = factories.DocumentFactory(link_reach="restricted") - factories.UserDocumentAccessFactory(document=document, user=user, role="owner") - - client = APIClient() - client.force_login(user) - - assert not cache.get(get_content_metadata_cache_key(document.id)) - - response = client.get( - f"/api/v1.0/documents/{document.id!s}/content/", - headers={ - "If-Modified-Since": timezone.now().strftime("%a, %d %b %Y %H:%M:%S %Z") - }, - ) - - assert response.status_code == status.HTTP_304_NOT_MODIFIED - - -def test_api_documents_content_retrieve_reusing_last_modified_since_invalid(): - """Fetching a content using a If-Modified-Since invalid should return a 200.""" - - user = factories.UserFactory() - document = factories.DocumentFactory(link_reach="restricted") - factories.UserDocumentAccessFactory(document=document, user=user, role="owner") - - client = APIClient() - client.force_login(user) - - file_metadata = default_storage.connection.meta.client.head_object( - Bucket=default_storage.bucket_name, Key=document.file_key - ) - last_modified = file_metadata["LastModified"] - etag = file_metadata["ETag"] - size = file_metadata["ContentLength"] - - cache.set( - get_content_metadata_cache_key(document.id), - { - "last_modified": last_modified.isoformat(), - "etag": etag, - "size": size, - }, - ) - - response = client.get( - f"/api/v1.0/documents/{document.id!s}/content/", - headers={ - "If-Modified-Since": (timezone.now() - timedelta(minutes=60)).strftime( - "%a, %d %b %Y %H:%M:%S %Z" - ) - }, - ) - - assert response.status_code == status.HTTP_200_OK - assert b"".join( - response.streaming_content - ) == factories.YDOC_HELLO_WORLD_BASE64.encode("utf-8") - assert response["Content-Length"] is not None - assert response["ETag"] is not None - assert response["Last-Modified"] is not None - assert response["Cache-Control"] == "private, no-cache" 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 deleted file mode 100644 index 79530d9cf..000000000 --- a/src/backend/core/tests/documents/test_api_documents_content_update.py +++ /dev/null @@ -1,271 +0,0 @@ -""" -Tests for the PATCH /api/v1.0/documents/{id}/content/ endpoint. -""" - -import base64 -from functools import cache -from uuid import uuid4 - -from django.core.files.storage import default_storage - -import pycrdt -import pytest -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()}, - ) - - 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}, - ) - - 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 403 for owners.""" - 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()}, - ) - - 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() - - -def test_api_documents_content_upadte_invalid_yjs_doc(): - """sending an invalid yjs doc as content should return a 400.""" - 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 - - response = client.patch( - f"/api/v1.0/documents/{document.id!s}/content/", - {"content": base64.b64encode(b"invalid yjs").decode("utf-8")}, - ) - - assert response.status_code == status.HTTP_400_BAD_REQUEST 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 b3105afe6..c4cd90b3a 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -51,8 +51,6 @@ def test_api_documents_retrieve_anonymous_public_standalone(): "public": ["reader", "commenter", "editor"], "restricted": None, }, - "content_patch": document.link_role == "editor", - "content_retrieve": True, "leave": False, "media_auth": True, "media_check": True, @@ -129,8 +127,6 @@ def test_api_documents_retrieve_anonymous_public_parent(): "link_select_options": models.LinkReachChoices.get_select_options( **links_definition ), - "content_patch": grand_parent.link_role == "editor", - "content_retrieve": True, "leave": False, "media_auth": True, "media_check": True, @@ -240,8 +236,6 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated( "public": ["reader", "commenter", "editor"], "restricted": None, }, - "content_patch": document.link_role == "editor", - "content_retrieve": True, "leave": True, "media_auth": True, "media_check": True, @@ -326,8 +320,6 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea **links_definition ), "move": False, - "content_patch": grand_parent.link_role == "editor", - "content_retrieve": True, "leave": True, "media_auth": True, "media_check": True, @@ -524,8 +516,6 @@ def test_api_documents_retrieve_authenticated_related_parent(): "link_select_options": models.LinkReachChoices.get_select_options( **link_definition ), - "content_patch": access.role not in ["reader", "commenter"], - "content_retrieve": True, "leave": access.role not in ["administrator", "owner"], "media_auth": True, "media_check": True, 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 b32da42e3..3a9caafec 100644 --- a/src/backend/core/tests/documents/test_api_documents_trashbin.py +++ b/src/backend/core/tests/documents/test_api_documents_trashbin.py @@ -100,8 +100,6 @@ def test_api_documents_trashbin_format(): "public": ["reader", "commenter", "editor"], "restricted": None, }, - "content_patch": False, - "content_retrieve": True, "leave": False, "media_auth": False, "media_check": False, @@ -167,8 +165,6 @@ def test_api_documents_trashbin_format(): "public": ["reader", "commenter", "editor"], "restricted": None, }, - "content_patch": False, - "content_retrieve": True, "leave": False, "media_auth": False, "media_check": False, 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 deleted file mode 100644 index a9d04c4ac..000000000 --- a/src/backend/core/tests/documents/test_api_documents_update_extract_attachments.py +++ /dev/null @@ -1,154 +0,0 @@ -""" -Test extract-attachments on document update in docs core app. -""" - -import base64 -from uuid import uuid4 - -import pycrdt -import pytest -from rest_framework.test import APIClient - -from core import factories - -pytestmark = pytest.mark.django_db - - -def get_ydoc_with_images(image_keys): - """Return a ydoc from text for testing purposes.""" - ydoc = pycrdt.Doc() - fragment = pycrdt.XmlFragment( - [ - pycrdt.XmlElement("img", {"src": f"http://localhost/media/{key:s}"}) - for key in image_keys - ] - ) - ydoc["document-store"] = fragment - update = ydoc.get_update() - return base64.b64encode(update).decode("utf-8") - - -def test_api_documents_update_new_attachment_keys_anonymous(django_assert_num_queries): - """ - When an anonymous user updates a document, the attachment keys extracted from the - updated content should be added to the list of "attachments" to the document if these - attachments are already readable by anonymous users. - """ - image_keys = [f"{uuid4()!s}/attachments/{uuid4()!s}.png" for _ in range(4)] - document = factories.DocumentFactory( - content=get_ydoc_with_images(image_keys[:1]), - attachments=[image_keys[0]], - link_reach="public", - link_role="editor", - ) - - factories.DocumentFactory(attachments=[image_keys[1]], link_reach="public") - factories.DocumentFactory(attachments=[image_keys[2]], link_reach="authenticated") - factories.DocumentFactory(attachments=[image_keys[3]], link_reach="restricted") - expected_keys = {image_keys[i] for i in [0, 1]} - - 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 == 204 - - document.refresh_from_db() - assert set(document.attachments) == expected_keys - - # 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().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 == 204 - - document.refresh_from_db() - assert len(document.attachments) == 2 - assert set(document.attachments) == expected_keys - - -def test_api_documents_update_new_attachment_keys_authenticated( - django_assert_num_queries, -): - """ - When an authenticated user updates a document, the attachment keys extracted from the - updated content should be added to the list of "attachments" to the document if these - attachments are already readable by the editing user. - """ - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - image_keys = [f"{uuid4()!s}/attachments/{uuid4()!s}.png" for _ in range(5)] - document = factories.DocumentFactory( - content=get_ydoc_with_images(image_keys[:1]), - attachments=[image_keys[0]], - users=[(user, "editor")], - ) - - factories.DocumentFactory(attachments=[image_keys[1]], link_reach="public") - factories.DocumentFactory(attachments=[image_keys[2]], link_reach="authenticated") - factories.DocumentFactory(attachments=[image_keys[3]], link_reach="restricted") - 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(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 == 204 - - document.refresh_from_db() - assert set(document.attachments) == expected_keys - - # 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.patch( - f"/api/v1.0/documents/{document.id!s}/content/", - {"content": get_ydoc_with_images(image_keys[:2])}, - format="json", - ) - assert response.status_code == 204 - - document.refresh_from_db() - assert len(document.attachments) == 4 - assert set(document.attachments) == expected_keys - - -def test_api_documents_update_new_attachment_keys_duplicate(): - """ - Duplicate keys in the content should not result in duplicates in the document's attachments. - """ - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - 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_images([image_key1]), - attachments=[image_key1], - users=[(user, "editor")], - ) - - factories.DocumentFactory(attachments=[image_key2], users=[user]) - - 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 == 204 - - document.refresh_from_db() - assert len(document.attachments) == 2 - assert set(document.attachments) == {image_key1, image_key2} diff --git a/src/backend/core/tests/test_api_utils_parse_http_conditional_headers.py b/src/backend/core/tests/test_api_utils_parse_http_conditional_headers.py deleted file mode 100644 index b654b9995..000000000 --- a/src/backend/core/tests/test_api_utils_parse_http_conditional_headers.py +++ /dev/null @@ -1,52 +0,0 @@ -""" -Unit tests for the parse_http_conditional_headers utility function. -""" - -import datetime as dt - -import pytest -from rest_framework.test import APIRequestFactory - -from core.api.utils import parse_http_conditional_headers - - -@pytest.fixture(name="prepare_request") -def fixture_prepare_request(request): - """ - Fixture returning a request with headers configured from the indirect parametrize parameters. - """ - return APIRequestFactory().get("/", headers=request.param) - - -@pytest.mark.parametrize( - "prepare_request, expected_if_none_match, expected_if_modified_since", - [ - ({}, None, None), - ({"if-none-match": '"abc123"'}, '"abc123"', None), - ({"if-none-match": 'W/"abc123"'}, '"abc123"', None), - ( - {"if-modified-since": "Wed, 21 Oct 2015 07:28:00 GMT"}, - None, - dt.datetime(2015, 10, 21, 7, 28, 0, tzinfo=dt.timezone.utc), - ), - ({"if-modified-since": "not-a-date"}, None, None), - ( - { - "if-none-match": 'W/"deadbeef"', - "if-modified-since": "Wed, 21 Oct 2015 07:28:00 GMT", - }, - '"deadbeef"', - dt.datetime(2015, 10, 21, 7, 28, 0, tzinfo=dt.timezone.utc), - ), - ], - indirect=["prepare_request"], -) -def test_api_utils_parse_http_conditional_headers( - prepare_request, expected_if_none_match, expected_if_modified_since -): - """Test parse_http_conditional_headers utils.""" - if_none_match, if_modified_since_dt = parse_http_conditional_headers( - prepare_request - ) - assert if_none_match == expected_if_none_match - assert if_modified_since_dt == expected_if_modified_since diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index 5c124d15f..407e239da 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -170,8 +170,6 @@ def test_models_documents_get_abilities_forbidden( "favorite": False, "comment": False, "invite_owner": False, - "content_patch": False, - "content_retrieve": False, "leave": False, "media_auth": False, "media_check": False, @@ -244,8 +242,6 @@ def test_models_documents_get_abilities_reader( "public": ["reader", "commenter", "editor"], "restricted": None, }, - "content_patch": False, - "content_retrieve": True, "leave": False, "media_auth": True, "media_check": True, @@ -317,8 +313,6 @@ def test_models_documents_get_abilities_commenter( "public": ["reader", "commenter", "editor"], "restricted": None, }, - "content_patch": False, - "content_retrieve": True, "leave": False, "media_auth": True, "media_check": True, @@ -387,8 +381,6 @@ def test_models_documents_get_abilities_editor( "public": ["reader", "commenter", "editor"], "restricted": None, }, - "content_patch": True, - "content_retrieve": True, "leave": False, "media_auth": True, "media_check": True, @@ -446,8 +438,6 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): "public": ["reader", "commenter", "editor"], "restricted": None, }, - "content_patch": True, - "content_retrieve": True, "leave": False, "media_auth": True, "media_check": True, @@ -491,8 +481,6 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): "public": ["reader", "commenter", "editor"], "restricted": None, }, - "content_patch": False, - "content_retrieve": True, "leave": False, "media_auth": False, "media_check": False, @@ -540,8 +528,6 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries) "public": ["reader", "commenter", "editor"], "restricted": None, }, - "content_patch": True, - "content_retrieve": True, "leave": False, "media_auth": True, "media_check": True, @@ -599,8 +585,6 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): "public": ["reader", "commenter", "editor"], "restricted": None, }, - "content_patch": True, - "content_retrieve": True, "leave": True, "media_auth": True, "media_check": True, @@ -666,8 +650,6 @@ def test_models_documents_get_abilities_reader_user( "public": ["reader", "commenter", "editor"], "restricted": None, }, - "content_patch": access_from_link, - "content_retrieve": True, "leave": True, "media_auth": True, "media_check": True, @@ -734,8 +716,6 @@ def test_models_documents_get_abilities_commenter_user( "public": ["reader", "commenter", "editor"], "restricted": None, }, - "content_patch": access_from_link, - "content_retrieve": True, "leave": True, "media_auth": True, "media_check": True, @@ -798,8 +778,6 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries): "public": ["reader", "commenter", "editor"], "restricted": None, }, - "content_patch": False, - "content_retrieve": True, "leave": True, "media_auth": True, "media_check": True, diff --git a/src/backend/core/tests/test_utils_s3_response_stream.py b/src/backend/core/tests/test_utils_s3_response_stream.py deleted file mode 100644 index e42c1d79f..000000000 --- a/src/backend/core/tests/test_utils_s3_response_stream.py +++ /dev/null @@ -1,125 +0,0 @@ -"""Test the s3 response stream utilities.""" - -from collections.abc import AsyncIterator, Iterator - -import pytest -from asgiref.sync import async_to_sync - -from core.utils.s3_response_stream import async_stream, content_stream, sync_stream - -pytestmark = pytest.mark.django_db - - -class FakeS3Body: - """Minimal stand-in for a botocore StreamingBody.""" - - def __init__(self, chunks): - self._chunks = chunks - self.closed = False - - def iter_chunks(self): - """Yield the configured chunks, like StreamingBody.iter_chunks.""" - yield from self._chunks - - def close(self): - """Record that the body has been closed.""" - self.closed = True - - -def collect_async(async_gen): - """Consume an async generator synchronously and return its items as a list.""" - - async def _collect(): - return [chunk async for chunk in async_gen] - - return async_to_sync(_collect)() - - -# -- sync_stream -- - - -def test_sync_stream_yields_all_chunks(): - """Should yield every chunk of the body in order.""" - body = FakeS3Body([b"hello", b"world", b"!"]) - - assert list(sync_stream(body)) == [b"hello", b"world", b"!"] - - -def test_sync_stream_empty_body(): - """Should yield nothing when the body is empty.""" - body = FakeS3Body([]) - - assert not list(sync_stream(body)) - - -def test_sync_stream_closes_body(): - """Should close the body once it has been fully consumed.""" - body = FakeS3Body([b"hello"]) - - assert body.closed is False - list(sync_stream(body)) - assert body.closed is True - - -# -- async_stream -- - - -def test_async_stream_yields_all_chunks(): - """Should yield every chunk of the body in order.""" - body = FakeS3Body([b"hello", b"world", b"!"]) - - assert collect_async(async_stream(body)) == [b"hello", b"world", b"!"] - - -def test_async_stream_empty_body(): - """Should yield nothing when the body is empty.""" - body = FakeS3Body([]) - - assert not collect_async(async_stream(body)) - - -def test_async_stream_closes_body(): - """Should close the body once it has been fully consumed.""" - body = FakeS3Body([b"hello"]) - - assert body.closed is False - collect_async(async_stream(body)) - assert body.closed is True - - -# -- content_stream -- - - -def test_content_stream_async_mode(monkeypatch): - """In async mode, content_stream should return an async iterator.""" - monkeypatch.setenv("PYTHON_SERVER_MODE", "async") - body = FakeS3Body([b"hello", b"world"]) - - stream = content_stream(body) - - assert isinstance(stream, AsyncIterator) - assert collect_async(stream) == [b"hello", b"world"] - - -def test_content_stream_sync_mode(monkeypatch): - """In sync mode, content_stream should return a sync iterator.""" - monkeypatch.setenv("PYTHON_SERVER_MODE", "sync") - body = FakeS3Body([b"hello", b"world"]) - - stream = content_stream(body) - - assert not isinstance(stream, AsyncIterator) - assert isinstance(stream, Iterator) - assert list(stream) == [b"hello", b"world"] - - -def test_content_stream_defaults_to_sync(monkeypatch): - """When PYTHON_SERVER_MODE is not set, content_stream should default to sync.""" - monkeypatch.delenv("PYTHON_SERVER_MODE", raising=False) - body = FakeS3Body([b"hello", b"world"]) - - stream = content_stream(body) - - assert not isinstance(stream, AsyncIterator) - assert isinstance(stream, Iterator) - assert list(stream) == [b"hello", b"world"] diff --git a/src/backend/core/utils/s3_response_stream.py b/src/backend/core/utils/s3_response_stream.py deleted file mode 100644 index ddf364f39..000000000 --- a/src/backend/core/utils/s3_response_stream.py +++ /dev/null @@ -1,47 +0,0 @@ -"""Utils module to stream content to a StreamingHttpResponse""" - -import os - -from asgiref.sync import sync_to_async -from botocore.response import StreamingBody - - -def _is_async_server(): - """ - Return whether the app runs as an ASGI application, based on the - PYTHON_SERVER_MODE environment variable (set in impress/asgi.py and - impress/wsgi.py). - """ - return os.environ.get("PYTHON_SERVER_MODE", "sync") == "async" - - -def sync_stream(body: StreamingBody): - """Synchronous generator consuming s3 response body.""" - yield from body.iter_chunks() - body.close() - - -async def async_stream(body: StreamingBody): - """Asynchronous generator consuming s3 response body""" - # The botocore stream is blocking, so each read is offloaded with - # sync_to_async to avoid blocking the event loop. - chunks = await sync_to_async(body.iter_chunks)() - sentinel = object() - while True: - chunk = await sync_to_async(next)(chunks, sentinel) - if chunk is sentinel: - break - yield chunk - await sync_to_async(body.close)() - - -def content_stream(body: StreamingBody): - """ - Depending on the server mode (set through the PYTHON_SERVER_MODE - environment variable in impress/asgi.py and impress/wsgi.py), the - content is streamed back with either an asynchronous or a synchronous - iterator. Under ASGI, a synchronous iterator would trigger a Django - warning and be consumed synchronously, defeating the purpose of - streaming. - """ - return async_stream(body) if _is_async_server() else sync_stream(body) diff --git a/src/backend/impress/settings.py b/src/backend/impress/settings.py index c3ac0dc82..d79ffb301 100755 --- a/src/backend/impress/settings.py +++ b/src/backend/impress/settings.py @@ -1132,10 +1132,6 @@ class Base(Configuration): ), } - CONTENT_METADATA_CACHE_TIMEOUT = values.IntegerValue( - 60 * 60 * 24, environ_name="CONTENT_METADATA_CACHE_TIMEOUT", environ_prefix=None - ) - TREEBEARD_PATH_COMPUTE_RETRY_MAX_ATTEMPTS = values.IntegerValue( 10, environ_name="TREEBEARD_PATH_COMPUTE_RETRY_MAX_ATTEMPTS",