fixup! ✨(back) add endpoint checking media status

We want to have the media-check url returned on the attachment-upload
response instead of the media url directly. The front will know the
endpoint to use to check the media status.

CHANGELOG.md

@@ -8,8 +8,9 @@ and this project adheres to
 
 ## [Unreleased]
 
-## Added
+### Added
 
+- ✨(back) add endpoint checking media status
 - ✨(backend) allow setting session cookie age via env var #977
 - ✨(backend) allow theme customnization using a configuration file #948
 - ✨ Add a custom callout block to the editor #892
@@ -18,7 +19,7 @@ and this project adheres to
 - 🩺(CI) add lint spell mistakes #954
 - 🛂(frontend) block edition to not connected users #945
 
-## Changed
+### Changed
 
 - 📝(frontend) Update documentation
 - ✅(frontend) Improve tests coverage

src/backend/core/api/viewsets.py

@@ -4,7 +4,7 @@
 import json
 import logging
 import uuid
-from urllib.parse import unquote, urlparse
+from urllib.parse import unquote, urlencode, urlparse
 
 from django.conf import settings
 from django.contrib.postgres.aggregates import ArrayAgg
@@ -18,6 +18,7 @@ from django.db import models as db
 from django.db.models.expressions import RawSQL
 from django.db.models.functions import Left, Length
 from django.http import Http404, StreamingHttpResponse
+from django.urls import reverse
 from django.utils.text import capfirst, slugify
 from django.utils.translation import gettext_lazy as _
 
@@ -1193,8 +1194,16 @@ class DocumentViewSet(
 
         malware_detection.analyse_file(key, document_id=document.id)
 
+        url = reverse(
+            "documents-media-check",
+            kwargs={"pk": document.id},
+        )
+        parameters = urlencode({"key": key})
+
         return drf.response.Response(
-            {"file": f"{settings.MEDIA_URL:s}{key:s}"},
+            {
+                "file": f"{url:s}?{parameters:s}",
+            },
             status=drf.status.HTTP_201_CREATED,
         )
 
@@ -1279,7 +1288,10 @@ class DocumentViewSet(
         # Check if the attachment is ready
         s3_client = default_storage.connection.meta.client
         bucket_name = default_storage.bucket_name
-        head_resp = s3_client.head_object(Bucket=bucket_name, Key=key)
+        try:
+            head_resp = s3_client.head_object(Bucket=bucket_name, Key=key)
+        except ClientError as err:
+            raise drf.exceptions.PermissionDenied() from err
         metadata = head_resp.get("Metadata", {})
         # In order to be compatible with existing upload without `status` metadata,
         # we consider them as ready.
@@ -1294,6 +1306,49 @@ class DocumentViewSet(
 
         return drf.response.Response("authorized", headers=request.headers, status=200)
 
+    @drf.decorators.action(detail=True, methods=["get"], url_path="media-check")
+    def media_check(self, request, *args, **kwargs):
+        """
+        Check if the media is ready to be served.
+        """
+        document = self.get_object()
+
+        key = request.query_params.get("key")
+        if not key:
+            return drf.response.Response(
+                {"detail": "Missing 'key' query parameter"},
+                status=drf.status.HTTP_400_BAD_REQUEST,
+            )
+
+        if key not in document.attachments:
+            return drf.response.Response(
+                {"detail": "Attachment missing"},
+                status=drf.status.HTTP_404_NOT_FOUND,
+            )
+
+        # Check if the attachment is ready
+        s3_client = default_storage.connection.meta.client
+        bucket_name = default_storage.bucket_name
+        try:
+            head_resp = s3_client.head_object(Bucket=bucket_name, Key=key)
+        except ClientError:
+            return drf.response.Response(
+                {"detail": "Media not found"},
+                status=drf.status.HTTP_404_NOT_FOUND,
+            )
+        metadata = head_resp.get("Metadata", {})
+
+        body = {
+            "status": metadata.get("status", enums.DocumentAttachmentStatus.PROCESSING),
+        }
+        if metadata.get("status") == enums.DocumentAttachmentStatus.READY:
+            body = {
+                "status": enums.DocumentAttachmentStatus.READY,
+                "file": f"{settings.MEDIA_URL:s}{key:s}",
+            }
+
+        return drf.response.Response(body, status=drf.status.HTTP_200_OK)
+
     @drf.decorators.action(
         detail=True,
         methods=["post"],

src/backend/core/models.py

@@ -835,6 +835,7 @@ class Document(MP_Node, BaseModel):
             "ai_transform": ai_access,
             "ai_translate": ai_access,
             "attachment_upload": can_update,
+            "media_check": can_get,
             "children_list": can_get,
             "children_create": can_update and user.is_authenticated,
             "collaboration_auth": can_get,

src/backend/core/tests/documents/test_api_documents_attachment_upload.py

@@ -5,6 +5,7 @@ Test file uploads API endpoint for users in impress's core app.
 import re
 import uuid
 from unittest import mock
+from urllib.parse import parse_qs, urlparse
 
 from django.core.files.storage import default_storage
 from django.core.files.uploadedfile import SimpleUploadedFile
@@ -66,8 +67,12 @@ def test_api_documents_attachment_upload_anonymous_success():
 
     assert response.status_code == 201
 
-    pattern = re.compile(rf"^/media/{document.id!s}/attachments/(.*)\.png")
-    file_path = response.json()["file"]
+    pattern = re.compile(rf"^{document.id!s}/attachments/(.*)\.png")
+    url_parsed = urlparse(response.json()["file"])
+    assert url_parsed.path == f"/api/v1.0/documents/{document.id!s}/media-check/"
+    query = parse_qs(url_parsed.query)
+    assert query["key"][0] is not None
+    file_path = query["key"][0]
     match = pattern.search(file_path)
     file_id = match.group(1)
     # Validate that file_id is a valid UUID
@@ -148,8 +153,13 @@ def test_api_documents_attachment_upload_authenticated_success(reach, role):
 
     assert response.status_code == 201
 
-    pattern = re.compile(rf"^/media/{document.id!s}/attachments/(.*)\.png")
-    match = pattern.search(response.json()["file"])
+    pattern = re.compile(rf"^{document.id!s}/attachments/(.*)\.png")
+    url_parsed = urlparse(response.json()["file"])
+    assert url_parsed.path == f"/api/v1.0/documents/{document.id!s}/media-check/"
+    query = parse_qs(url_parsed.query)
+    assert query["key"][0] is not None
+    file_path = query["key"][0]
+    match = pattern.search(file_path)
     file_id = match.group(1)
 
     mock_analyse_file.assert_called_once_with(
@@ -224,8 +234,12 @@ def test_api_documents_attachment_upload_success(via, role, mock_user_teams):
 
     assert response.status_code == 201
 
-    file_path = response.json()["file"]
-    pattern = re.compile(rf"^/media/{document.id!s}/attachments/(.*)\.png")
+    pattern = re.compile(rf"^{document.id!s}/attachments/(.*)\.png")
+    url_parsed = urlparse(response.json()["file"])
+    assert url_parsed.path == f"/api/v1.0/documents/{document.id!s}/media-check/"
+    query = parse_qs(url_parsed.query)
+    assert query["key"][0] is not None
+    file_path = query["key"][0]
     match = pattern.search(file_path)
     file_id = match.group(1)
 
@@ -320,8 +334,13 @@ def test_api_documents_attachment_upload_fix_extension(
 
     assert response.status_code == 201
 
-    file_path = response.json()["file"]
-    pattern = re.compile(rf"^/media/{document.id!s}/attachments/(.*)\.{extension:s}")
+    pattern = re.compile(rf"^{document.id!s}/attachments/(.*)\.{extension:s}")
+    url_parsed = urlparse(response.json()["file"])
+    assert url_parsed.path == f"/api/v1.0/documents/{document.id!s}/media-check/"
+    query = parse_qs(url_parsed.query)
+    assert query["key"][0] is not None
+    file_path = query["key"][0]
+
     match = pattern.search(file_path)
     file_id = match.group(1)
 
@@ -386,8 +405,12 @@ def test_api_documents_attachment_upload_unsafe():
 
     assert response.status_code == 201
 
-    file_path = response.json()["file"]
-    pattern = re.compile(rf"^/media/{document.id!s}/attachments/(.*)\.exe")
+    pattern = re.compile(rf"^{document.id!s}/attachments/(.*)\.exe")
+    url_parsed = urlparse(response.json()["file"])
+    assert url_parsed.path == f"/api/v1.0/documents/{document.id!s}/media-check/"
+    query = parse_qs(url_parsed.query)
+    assert query["key"][0] is not None
+    file_path = query["key"][0]
     match = pattern.search(file_path)
     file_id = match.group(1)
 

src/backend/core/tests/documents/test_api_documents_media_check.py

@@ -0,0 +1,244 @@
+"""Test the "media_check" endpoint."""
+
+from io import BytesIO
+from uuid import uuid4
+
+from django.core.files.storage import default_storage
+
+import pytest
+from rest_framework.test import APIClient
+
+from core import factories
+from core.enums import DocumentAttachmentStatus
+from core.tests.conftest import TEAM, USER, VIA
+
+pytestmark = pytest.mark.django_db
+
+
+def test_api_documents_media_check_unknown_document():
+    """
+    The "media_check" endpoint should return a 404 error if the document does not exist.
+    """
+    client = APIClient()
+    response = client.get(f"/api/v1.0/documents/{uuid4()!s}media-check/")
+    assert response.status_code == 404
+
+
+def test_api_documents_media_check_missing_key():
+    """
+    The "media_check" endpoint should return a 404 error if the key is missing.
+    """
+    user = factories.UserFactory()
+
+    client = APIClient()
+    client.force_login(user=user)
+
+    document = factories.DocumentFactory(users=[user])
+
+    response = client.get(f"/api/v1.0/documents/{document.id!s}/media-check/")
+    assert response.status_code == 400
+    assert response.json() == {"detail": "Missing 'key' query parameter"}
+
+
+def test_api_documents_media_check_key_parameter_not_related_to_document():
+    """
+    The "media_check" endpoint should return a 404 error if the key is not related to the document.
+    """
+    user = factories.UserFactory()
+
+    client = APIClient()
+    client.force_login(user=user)
+
+    document = factories.DocumentFactory(users=[user])
+
+    response = client.get(
+        f"/api/v1.0/documents/{document.id!s}/media-check/",
+        {"key": f"{document.id!s}/attachments/unknown.jpg"},
+    )
+    assert response.status_code == 404
+    assert response.json() == {"detail": "Attachment missing"}
+
+
+def test_api_documents_media_check_anonymous_public_document():
+    """
+    The "media_check" endpoint should return a 200 status code if the document is public.
+    """
+    document = factories.DocumentFactory(link_reach="public")
+
+    filename = f"{uuid4()!s}.jpg"
+    key = f"{document.id!s}/attachments/{filename:s}"
+    default_storage.connection.meta.client.put_object(
+        Bucket=default_storage.bucket_name,
+        Key=key,
+        Body=BytesIO(b"my prose"),
+        ContentType="text/plain",
+        Metadata={"status": DocumentAttachmentStatus.PROCESSING},
+    )
+    document.attachments = [key]
+    document.save(update_fields=["attachments"])
+
+    client = APIClient()
+
+    response = client.get(
+        f"/api/v1.0/documents/{document.id!s}/media-check/", {"key": key}
+    )
+    assert response.status_code == 200
+    assert response.json() == {"status": DocumentAttachmentStatus.PROCESSING}
+
+
+def test_api_documents_media_check_anonymous_public_document_ready():
+    """
+    The "media_check" endpoint should return a 200 status code if the document is public.
+    """
+    document = factories.DocumentFactory(link_reach="public")
+
+    filename = f"{uuid4()!s}.jpg"
+    key = f"{document.id!s}/attachments/{filename:s}"
+    default_storage.connection.meta.client.put_object(
+        Bucket=default_storage.bucket_name,
+        Key=key,
+        Body=BytesIO(b"my prose"),
+        ContentType="text/plain",
+        Metadata={"status": DocumentAttachmentStatus.READY},
+    )
+    document.attachments = [key]
+    document.save(update_fields=["attachments"])
+
+    client = APIClient()
+
+    response = client.get(
+        f"/api/v1.0/documents/{document.id!s}/media-check/", {"key": key}
+    )
+    assert response.status_code == 200
+    assert response.json() == {
+        "status": DocumentAttachmentStatus.READY,
+        "file": f"/media/{key:s}",
+    }
+
+
+@pytest.mark.parametrize("link_reach", ["restricted", "authenticated"])
+def test_api_documents_media_check_anonymous_non_public_document(link_reach):
+    """
+    The "media_check" endpoint should return a 403 error if the document is not public.
+    """
+    document = factories.DocumentFactory(link_reach=link_reach)
+
+    client = APIClient()
+
+    response = client.get(f"/api/v1.0/documents/{document.id!s}/media-check/")
+    assert response.status_code == 401
+
+
+def test_api_documents_media_check_connected_document():
+    """
+    The "media_check" endpoint should return a 200 status code for a user connected
+    checking for a document with link_reach authenticated.
+    """
+    document = factories.DocumentFactory(link_reach="authenticated")
+
+    filename = f"{uuid4()!s}.jpg"
+    key = f"{document.id!s}/attachments/{filename:s}"
+    default_storage.connection.meta.client.put_object(
+        Bucket=default_storage.bucket_name,
+        Key=key,
+        Body=BytesIO(b"my prose"),
+        ContentType="text/plain",
+        Metadata={"status": DocumentAttachmentStatus.READY},
+    )
+    document.attachments = [key]
+    document.save(update_fields=["attachments"])
+
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user=user)
+
+    response = client.get(
+        f"/api/v1.0/documents/{document.id!s}/media-check/", {"key": key}
+    )
+    assert response.status_code == 200
+    assert response.json() == {
+        "status": DocumentAttachmentStatus.READY,
+        "file": f"/media/{key:s}",
+    }
+
+
+def test_api_documents_media_check_connected_document_media_not_related():
+    """
+    The "media_check" endpoint should return a 404 error if the key is not related to the document.
+    """
+    document = factories.DocumentFactory(link_reach="authenticated")
+
+    filename = f"{uuid4()!s}.jpg"
+    key = f"{document.id!s}/attachments/{filename:s}"
+
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user=user)
+
+    response = client.get(
+        f"/api/v1.0/documents/{document.id!s}/media-check/", {"key": key}
+    )
+    assert response.status_code == 404
+    assert response.json() == {"detail": "Attachment missing"}
+
+
+def test_api_documents_media_check_media_missing_on_storage():
+    """
+    The "media_check" endpoint should return a 404 error if the media is missing on storage.
+    """
+    document = factories.DocumentFactory(link_reach="authenticated")
+
+    filename = f"{uuid4()!s}.jpg"
+    key = f"{document.id!s}/attachments/{filename:s}"
+
+    document.attachments = [key]
+    document.save(update_fields=["attachments"])
+
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user=user)
+
+    response = client.get(
+        f"/api/v1.0/documents/{document.id!s}/media-check/", {"key": key}
+    )
+    assert response.status_code == 404
+    assert response.json() == {"detail": "Media not found"}
+
+
+@pytest.mark.parametrize("via", VIA)
+def test_api_documents_media_check_restricted_document(via, mock_user_teams):
+    """
+    The "media_check" endpoint should return a 200 status code if the document is restricted and
+    the user has access to it.
+    """
+    document = factories.DocumentFactory(link_reach="restricted")
+    filename = f"{uuid4()!s}.jpg"
+    key = f"{document.id!s}/attachments/{filename:s}"
+    default_storage.connection.meta.client.put_object(
+        Bucket=default_storage.bucket_name,
+        Key=key,
+        Body=BytesIO(b"my prose"),
+        ContentType="text/plain",
+        Metadata={"status": DocumentAttachmentStatus.READY},
+    )
+    document.attachments = [key]
+    document.save(update_fields=["attachments"])
+
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user=user)
+
+    if via == USER:
+        factories.UserDocumentAccessFactory(document=document, user=user)
+    elif via == TEAM:
+        mock_user_teams.return_value = ["lasuite", "unknown"]
+        factories.TeamDocumentAccessFactory(document=document, team="lasuite")
+
+    response = client.get(
+        f"/api/v1.0/documents/{document.id!s}/media-check/", {"key": key}
+    )
+    assert response.status_code == 200
+    assert response.json() == {
+        "status": DocumentAttachmentStatus.READY,
+        "file": f"/media/{key:s}",
+    }

src/backend/core/tests/documents/test_api_documents_retrieve.py

@@ -48,6 +48,7 @@ def test_api_documents_retrieve_anonymous_public_standalone():
                 "restricted": ["reader", "editor"],
             },
             "media_auth": True,
+            "media_check": True,
             "move": False,
             "partial_update": document.link_role == "editor",
             "restore": False,
@@ -111,6 +112,7 @@ def test_api_documents_retrieve_anonymous_public_parent():
             "link_configuration": False,
             "link_select_options": models.LinkReachChoices.get_select_options(links),
             "media_auth": True,
+            "media_check": True,
             "move": False,
             "partial_update": grand_parent.link_role == "editor",
             "restore": False,
@@ -210,6 +212,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated(
                 "restricted": ["reader", "editor"],
             },
             "media_auth": True,
+            "media_check": True,
             "move": False,
             "partial_update": document.link_role == "editor",
             "restore": False,
@@ -279,8 +282,9 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea
             "invite_owner": False,
             "link_configuration": False,
             "link_select_options": models.LinkReachChoices.get_select_options(links),
-            "move": False,
             "media_auth": True,
+            "media_check": True,
+            "move": False,
             "partial_update": grand_parent.link_role == "editor",
             "restore": False,
             "retrieve": True,
@@ -460,6 +464,7 @@ def test_api_documents_retrieve_authenticated_related_parent():
             "link_configuration": access.role in ["administrator", "owner"],
             "link_select_options": models.LinkReachChoices.get_select_options(links),
             "media_auth": True,
+            "media_check": True,
             "move": access.role in ["administrator", "owner"],
             "partial_update": access.role != "reader",
             "restore": access.role == "owner",

src/backend/core/tests/documents/test_api_documents_trashbin.py

@@ -91,6 +91,7 @@ def test_api_documents_trashbin_format():
                 "restricted": ["reader", "editor"],
             },
             "media_auth": True,
+            "media_check": True,
             "move": False,  # Can't move a deleted document
             "partial_update": True,
             "restore": True,

src/backend/core/tests/test_models_documents.py

@@ -165,6 +165,7 @@ def test_models_documents_get_abilities_forbidden(
         "favorite": False,
         "invite_owner": False,
         "media_auth": False,
+        "media_check": False,
         "move": False,
         "link_configuration": False,
         "link_select_options": {
@@ -231,6 +232,7 @@ def test_models_documents_get_abilities_reader(
             "restricted": ["reader", "editor"],
         },
         "media_auth": True,
+        "media_check": True,
         "move": False,
         "partial_update": False,
         "restore": False,
@@ -293,6 +295,7 @@ def test_models_documents_get_abilities_editor(
             "restricted": ["reader", "editor"],
         },
         "media_auth": True,
+        "media_check": True,
         "move": False,
         "partial_update": True,
         "restore": False,
@@ -344,6 +347,7 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries):
             "restricted": ["reader", "editor"],
         },
         "media_auth": True,
+        "media_check": True,
         "move": True,
         "partial_update": True,
         "restore": True,
@@ -392,6 +396,7 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries)
             "restricted": ["reader", "editor"],
         },
         "media_auth": True,
+        "media_check": True,
         "move": True,
         "partial_update": True,
         "restore": False,
@@ -443,6 +448,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries):
             "restricted": ["reader", "editor"],
         },
         "media_auth": True,
+        "media_check": True,
         "move": False,
         "partial_update": True,
         "restore": False,
@@ -501,6 +507,7 @@ def test_models_documents_get_abilities_reader_user(
             "restricted": ["reader", "editor"],
         },
         "media_auth": True,
+        "media_check": True,
         "move": False,
         "partial_update": access_from_link,
         "restore": False,
@@ -557,6 +564,7 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries):
             "restricted": ["reader", "editor"],
         },
         "media_auth": True,
+        "media_check": True,
         "move": False,
         "partial_update": False,
         "restore": False,