diff --git a/services/dcs/src/dcs/adapters/inbound/fastapi_/routes.py b/services/dcs/src/dcs/adapters/inbound/fastapi_/routes.py index ad30031..eca6f91 100644 --- a/services/dcs/src/dcs/adapters/inbound/fastapi_/routes.py +++ b/services/dcs/src/dcs/adapters/inbound/fastapi_/routes.py @@ -107,11 +107,8 @@ async def get_drs_object( if not work_order_context.file_id == object_id: raise http_exceptions.HttpWrongFileAuthorizationError() - # Instruct clients to refresh their download URL shortly before it expires expires_after = config.presigned_url_expires_after - buffer = config.url_expiration_buffer - url_max_age = max(buffer, expires_after - buffer) - cache_control_header = {"Cache-Control": f"max-age={url_max_age}"} + cache_control_header = {"Cache-Control": f"max-age={expires_after}"} try: drs_object = await data_repository.access_drs_object(drs_id=object_id) return Response( diff --git a/services/dcs/src/dcs/core/data_repository.py b/services/dcs/src/dcs/core/data_repository.py index 4dc3573..a277633 100644 --- a/services/dcs/src/dcs/core/data_repository.py +++ b/services/dcs/src/dcs/core/data_repository.py @@ -28,7 +28,7 @@ S3ObjectStoragesConfig, ) from hexkit.protocols.objstorage import ObjectStorageProtocol -from pydantic import Field, PositiveInt, field_validator, model_validator +from pydantic import Field, PositiveInt, field_validator from pydantic_settings import BaseSettings from dcs.adapters.outbound.http import exceptions @@ -89,14 +89,6 @@ class DataRepositoryConfig(BaseSettings): title="Presigned URL expiration time in seconds", examples=[30, 60], ) - url_expiration_buffer: PositiveInt = Field( - default=5, - description="Buffer time in seconds before the presigned URL expires, used to" - + " instruct clients to refresh their download URL shortly before it expires." - + " Should be less than presigned_url_expires_after.", - title="Download URL expiration buffer time (sec)", - examples=[5, 10], - ) cache_timeout: int = Field( default=7, description="Time in days since last access after which a file present in the " @@ -118,19 +110,6 @@ def check_server_uri(cls, value: str): return value - @model_validator(mode="after") - def check_url_expiration_buffer(self): - """Check that the buffer is less than the expiration time.""" - value = self.url_expiration_buffer - if value >= self.presigned_url_expires_after: - message = ( - "url_expiration_buffer must be less than presigned_url_expires_after" - + f", got: {value} (should be less than" - + f" {self.presigned_url_expires_after})" - ) - raise ValueError(message) - return self - class DataRepository(DataRepositoryPort): """A service that manages a registry of DRS objects.""" diff --git a/services/dcs/tests_dcs/test_edge_cases.py b/services/dcs/tests_dcs/test_edge_cases.py index 66678d9..c0a35fa 100644 --- a/services/dcs/tests_dcs/test_edge_cases.py +++ b/services/dcs/tests_dcs/test_edge_cases.py @@ -17,20 +17,15 @@ import re from dataclasses import dataclass -from unittest.mock import AsyncMock import httpx import pytest import pytest_asyncio from fastapi import status -from ghga_service_commons.api.testing import AsyncTestClient from ghga_service_commons.utils.utc_dates import now_as_utc from pytest_httpx import HTTPXMock, httpx_mock # noqa: F401 -from dcs.config import Config from dcs.core import models -from dcs.core.data_repository import DataRepositoryConfig -from dcs.inject import prepare_rest_app from dcs.ports.outbound.dao import DrsObjectDaoPort from tests_dcs.fixtures.joint import EXAMPLE_FILE, JointFixture, PopulatedFixture from tests_dcs.fixtures.mock_api.app import router @@ -196,61 +191,3 @@ async def test_register_file_twice(populated_fixture: PopulatedFixture, caplog): failure_message = f"Could not register file with id '{ example_file.file_id}' as an entry already exists for this id." assert failure_message in caplog.messages - - -@pytest.mark.parametrize( - "url_validity, buffer, expected_url_max_age", - [(60, 10, 50), (15, 10, 10)], - ids=["Normal", "UseBufferAsMinimum"], -) -async def test_cache_headers( - joint_fixture: JointFixture, - url_validity: int, - buffer: int, - expected_url_max_age: int, - monkeypatch, -): - """Test that the cache headers are set correctly""" - joint_config = joint_fixture.config.model_dump() - joint_config.update( - presigned_url_expires_after=url_validity, - url_expiration_buffer=buffer, - ) - config = Config(**joint_config) - work_order_token = generate_work_order_token( - file_id="test-file", - jwk=joint_fixture.jwk, - valid_seconds=120, - ) - headers = httpx.Headers({"Authorization": f"Bearer {work_order_token}"}) - monkeypatch.setattr( - joint_fixture.data_repository, - "access_drs_object", - AsyncMock(return_value=EXAMPLE_FILE), - ) - async with prepare_rest_app( - config=config, data_repo_override=joint_fixture.data_repository - ) as app: - async with AsyncTestClient(app) as client: - response = await client.get("/objects/test-file", headers=headers) - - assert "Cache-Control" in response.headers - assert response.headers["Cache-Control"] == f"max-age={expected_url_max_age}" - - -async def test_cache_param_validation(): - """Test that the http cache-related config parameters are validated correctly""" - with pytest.raises(ValueError): - DataRepositoryConfig( - drs_server_uri="", - ekss_base_url="", - presigned_url_expires_after=0, - url_expiration_buffer=10, - ) - with pytest.raises(ValueError): - DataRepositoryConfig( - drs_server_uri="", - ekss_base_url="", - presigned_url_expires_after=10, - url_expiration_buffer=10, - ) diff --git a/services/dcs/tests_dcs/test_typical_journey.py b/services/dcs/tests_dcs/test_typical_journey.py index 1806d6c..e7e8e41 100644 --- a/services/dcs/tests_dcs/test_typical_journey.py +++ b/services/dcs/tests_dcs/test_typical_journey.py @@ -139,7 +139,7 @@ async def test_happy_journey( assert "Cache-Control" in drs_object_response.headers assert ( drs_object_response.headers["Cache-Control"] - == f"max-age={joint_fixture.config.presigned_url_expires_after - 5}" + == f"max-age={joint_fixture.config.presigned_url_expires_after}" ) # download file bytes: