From 021f53092eb190cf20a8cedd4903fa9fb6f3a81e Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Wed, 10 Jun 2026 08:36:03 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(backend)=20stream=20empty=20string?= =?UTF-8?q?=20with=20an=20async=20iterator=20under=20ASGI?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the content_retrieve action, if the document is not existing on the object storage, we return an empty binary string. In the case the app is ran as an ASGI application we still have a warning explaining it must consume the iterator before sending it. --- src/backend/core/api/viewsets.py | 6 ++- .../test_api_documents_content_retrieve.py | 39 ++++++++++++++++++- src/backend/core/utils/s3_response_stream.py | 13 +++++-- 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index b5d4304af..a01e948a5 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -10,6 +10,7 @@ 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 @@ -35,6 +36,7 @@ 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 @@ -2143,7 +2145,9 @@ class DocumentViewSet( return drf_response.Response(status=status.HTTP_304_NOT_MODIFIED) case "NoSuchKey" | "404": return StreamingHttpResponse( - b"", content_type="text/plain", status=200 + content_stream(StreamingBody(BytesIO(b""), content_length=0)), + content_type="text/plain", + status=200, ) case _: raise 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 index 59bd40f54..3d1ce1c3b 100644 --- a/src/backend/core/tests/documents/test_api_documents_content_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_content_retrieve.py @@ -141,7 +141,7 @@ def test_api_documents_content_retrieve_nonexistent_document(): def test_api_documents_content_retrieve_file_not_in_storage(): - """Returns an empty string when the file does not exists on the 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") @@ -188,13 +188,48 @@ async def test_api_documents_content_retrieve_async(monkeypatch): assert response.status_code == status.HTTP_200_OK # Wait for the streaming content to be fully received => async iterator -> list - # This fails it the streaming is not an async generator + # 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() diff --git a/src/backend/core/utils/s3_response_stream.py b/src/backend/core/utils/s3_response_stream.py index a029a356a..ddf364f39 100644 --- a/src/backend/core/utils/s3_response_stream.py +++ b/src/backend/core/utils/s3_response_stream.py @@ -6,6 +6,15 @@ 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() @@ -35,6 +44,4 @@ def content_stream(body: StreamingBody): warning and be consumed synchronously, defeating the purpose of streaming. """ - is_async_server = os.environ.get("PYTHON_SERVER_MODE", "sync") == "async" - - return async_stream(body) if is_async_server else sync_stream(body) + return async_stream(body) if _is_async_server() else sync_stream(body)