mirror of
https://github.com/suitenumerique/docs.git
synced 2026-09-12 04:37:53 +02:00
🐛(backend) stream empty string with an async iterator under ASGI
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user