diff --git a/src/backend/core/api/utils.py b/src/backend/core/api/utils.py index 3a78bccbb..7f62ab88a 100644 --- a/src/backend/core/api/utils.py +++ b/src/backend/core/api/utils.py @@ -194,3 +194,8 @@ 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}" diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 70c15aee6..2c64e3959 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -3,6 +3,7 @@ # pylint: disable=too-many-lines import base64 +import datetime as dt import ipaddress import json import logging @@ -1908,16 +1909,71 @@ class DocumentViewSet( 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) - def _content_retrieve(self, document): + def _content_retrieve(self, request, document): """Retrieve the raw content file ni s3 and stream it.""" - - if not default_storage.exists(document.file_key): - return StreamingHttpResponse( - b"", content_type="text/plain", status=status.HTTP_200_OK + if not ( + content_metadata := cache.get( + utils.get_content_metadata_cache_key(document.id) ) + ): + try: + file_metadata = default_storage.connection.meta.client.head_object( + Bucket=default_storage.bucket_name, Key=document.file_key + ) + except ClientError: + return StreamingHttpResponse( + b"", content_type="text/plain", status=status.HTTP_200_OK + ) + + last_modified = file_metadata["LastModified"] + etag = file_metadata["ETag"] + size = file_metadata["ContentLength"] + + cache.set( + utils.get_content_metadata_cache_key(document.id), + { + "last_modified": last_modified.isoformat(), + "etag": etag, + "size": size, + }, + settings.CONTENT_METADATA_CACHE_TIMEOUT, + ) + else: + last_modified = dt.datetime.fromisoformat( + content_metadata.get("last_modified") + ) + etag = content_metadata.get("etag") + size = content_metadata.get("size") + + # --- Check conditional headers from any client --- + if_none_match = request.META.get("HTTP_IF_NONE_MATCH") # contains ETag + if_modified_since = request.META.get("HTTP_IF_MODIFIED_SINCE") + + # Strip the W/ weak prefix. Proxies (e.g. nginx with gzip) convert strong + # ETags to weak ones, so a strict equality check would fail on production + # even when unchanged. + if if_none_match and if_none_match.startswith("W/"): + if_none_match = if_none_match.removeprefix("W/") + + if if_none_match and if_none_match == etag: + return drf_response.Response(status=status.HTTP_304_NOT_MODIFIED) + + if if_modified_since: + try: + since = dt.datetime.strptime( + if_modified_since, "%a, %d %b %Y %H:%M:%S %Z" + ) + except ValueError: + pass + else: + if not since.tzinfo: + since = since.replace(tzinfo=dt.timezone.utc) + if last_modified <= since: + return drf_response.Response(status=status.HTTP_304_NOT_MODIFIED) def _stream(file_key): with default_storage.open(file_key, "rb") as f: @@ -1930,10 +1986,10 @@ class DocumentViewSet( status=status.HTTP_200_OK, ) - try: - response["Content-Length"] = default_storage.size(document.file_key) - except NotImplementedError: - pass + 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 @@ -1951,7 +2007,7 @@ class DocumentViewSet( # to prevent having a massive number of database connections during # the web-socket re-connection burst. connection.close() - return self._content_retrieve(document) + return self._content_retrieve(request, document) return drf_response.Response(status=status.HTTP_501_NOT_IMPLEMENTED) 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 8b9331aa5..a4e8ae4b9 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 @@ -2,15 +2,19 @@ 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 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 @@ -30,6 +34,8 @@ 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 @@ -37,6 +43,12 @@ def test_api_documents_content_retrieve_anonymous_public(): 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(): @@ -64,12 +76,20 @@ def test_api_documents_content_retrieve_authenticated_not_restricted(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) @@ -92,12 +112,20 @@ def test_api_documents_content_retrieve_success(role, via, mock_user_teams): 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(): @@ -128,6 +156,12 @@ def test_api_documents_content_retrieve_file_not_in_storage(): 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)) def test_api_documents_content_retrieve_content_length_header(): @@ -182,9 +216,225 @@ def test_api_documents_content_retrieve_deleted_document_for_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/") 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/impress/settings.py b/src/backend/impress/settings.py index aea0b45d4..d1bd723da 100755 --- a/src/backend/impress/settings.py +++ b/src/backend/impress/settings.py @@ -18,6 +18,7 @@ from django.utils.translation import gettext_lazy as _ import sentry_sdk from configurations import Configuration, values +from corsheaders.defaults import default_headers from csp.constants import NONE from lasuite.configuration.values import SecretFileValue from sentry_sdk.integrations.django import DjangoIntegration @@ -1048,6 +1049,10 @@ class Base(Configuration): ), } + CONTENT_METADATA_CACHE_TIMEOUT = values.IntegerValue( + 60 * 60 * 24, environ_name="CONTENT_METADATA_CACHE_TIMEOUT", environ_prefix=None + ) + # pylint: disable=invalid-name @property def ENVIRONMENT(self): @@ -1170,6 +1175,12 @@ class Development(Base): ALLOWED_HOSTS = ["*"] CORS_ALLOW_ALL_ORIGINS = True CSRF_TRUSTED_ORIGINS = ["http://localhost:8072", "http://localhost:3000"] + CORS_ALLOW_HEADERS = ( + *default_headers, + "if-none-match", + "if-modified-since", + ) + CORS_EXPOSE_HEADERS = ["ETag"] DEBUG = True USE_SWAGGER = True