Skip to content

Commit

Permalink
Remove cache buffer thing
Browse files Browse the repository at this point in the history
Make the client use something like min-fresh in requests for ensuring a good buffer
  • Loading branch information
TheByronHimes committed Dec 24, 2024
1 parent 2a1a068 commit a18d00c
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 90 deletions.
5 changes: 1 addition & 4 deletions services/dcs/src/dcs/adapters/inbound/fastapi_/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
23 changes: 1 addition & 22 deletions services/dcs/src/dcs/core/data_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 "
Expand All @@ -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."""
Expand Down
63 changes: 0 additions & 63 deletions services/dcs/tests_dcs/test_edge_cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
)
2 changes: 1 addition & 1 deletion services/dcs/tests_dcs/test_typical_journey.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit a18d00c

Please sign in to comment.