Skip to content

fix(api): apply default cache expiry to posted vcons #162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
howethomas wants to merge 1 commit into
base: merge/refactor-with-tests
Choose a base branch
from
Open

fix(api): apply default cache expiry to posted vcons #162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions api/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,12 @@ async def add_vcon_to_set(vcon_uuid: str, timestamp: int) -> None:
await redis_async.zadd(VCON_SORTED_SET_NAME, {vcon_uuid: timestamp})


async def cache_vcon_in_redis(vcon_key: str, vcon: dict) -> None:
"""Store a vCon in Redis and apply the default cache expiry."""
await redis_async.json().set(vcon_key, "$", vcon)
await redis_async.expire(vcon_key, VCON_REDIS_EXPIRY)


async def ensure_vcon_in_redis(vcon_uuid: UUID) -> Optional[dict]:
"""Ensure a vCon exists in Redis, syncing from storage if necessary.

Expand Down Expand Up @@ -475,8 +481,7 @@ async def sync_vcon_from_storage(vcon_uuid: UUID) -> Optional[dict]:
vcon = Storage(storage_name=storage_name).get(str(vcon_uuid))
if vcon:
# Store the vCon back in Redis with expiration
await redis_async.json().set(f"vcon:{str(vcon_uuid)}", "$", vcon)
await redis_async.expire(f"vcon:{str(vcon_uuid)}", VCON_REDIS_EXPIRY)
await cache_vcon_in_redis(f"vcon:{str(vcon_uuid)}", vcon)
# Add to sorted set for timestamp-based retrieval
created_at = datetime.fromisoformat(vcon["created_at"])
timestamp = int(created_at.timestamp())
Expand Down Expand Up @@ -760,7 +765,7 @@ async def post_vcon(
timestamp = int(created_at.timestamp())

logger.debug(f"Storing vCon {inbound_vcon.uuid} ({len(dict_vcon)} bytes)")
await redis_async.json().set(key, "$", dict_vcon)
await cache_vcon_in_redis(key, dict_vcon)

logger.debug(f"Adding vCon {inbound_vcon.uuid} to sorted set")
await add_vcon_to_set(key, timestamp)
Expand Down Expand Up @@ -860,7 +865,7 @@ async def external_ingress_vcon(
logger.debug(
f"Storing vCon {inbound_vcon.uuid} ({len(dict_vcon)} bytes) via external ingress"
)
await redis_async.json().set(key, "$", dict_vcon)
await cache_vcon_in_redis(key, dict_vcon)

logger.debug(f"Adding vCon {inbound_vcon.uuid} to sorted set")
await add_vcon_to_set(key, timestamp)
Expand Down
40 changes: 11 additions & 29 deletions common/tests/test_post_vcon_expiry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
"""Unit tests for POST vCon expiry behavior.

These tests verify that vCons created via POST endpoints are stored and
indexed. As of the optimize-ingest-indexing change, default VCON_REDIS_EXPIRY
TTL is no longer set on newly stored vCons; retention is controlled by
storage backends or explicit TTL. Indexing (index_vcon_parties) still uses
Redis sadd/expire for party index keys.
indexed with the default `VCON_REDIS_EXPIRY` TTL applied to the vCon key.
Indexing (index_vcon_parties) still uses Redis sadd/expire for party index keys.
"""

from unittest.mock import AsyncMock, MagicMock, patch

import pytest
import api
from fastapi.testclient import TestClient
from settings import CONSERVER_API_TOKEN, CONSERVER_HEADER_NAME, VCON_REDIS_EXPIRY
Expand All @@ -29,11 +26,10 @@ def setup_method(self):

@patch("api.add_vcon_to_set")
@patch("api.index_vcon_parties")
def test_post_vcon_stores_without_default_expiry(
def test_post_vcon_stores_with_default_expiry(
self, mock_index_vcon_parties, mock_add_vcon_to_set
):
"""Test that POST /vcon stores vCon and indexes parties; no default vcon TTL."""
# Mock Redis client (sadd/expire used by index_vcon_parties when not patched)
"""Test that POST /vcon stores the vCon with the default cache TTL."""
mock_redis = MagicMock()
mock_json = MagicMock()
mock_json.set = AsyncMock()
Expand All @@ -56,13 +52,8 @@ def test_post_vcon_stores_without_default_expiry(

assert response.status_code == 201
mock_json.set.assert_called_once()
# No default TTL on vcon key: expire is only used for index keys by index_vcon_parties
vcon_key = f"vcon:{self.test_vcon['uuid']}"
vcon_expire_calls = [
c for c in mock_redis.expire.call_args_list
if c[0][0] == vcon_key and c[0][1] == VCON_REDIS_EXPIRY
]
assert len(vcon_expire_calls) == 0, "vCon key should not get default VCON_REDIS_EXPIRY TTL"
mock_redis.expire.assert_awaited_once_with(vcon_key, VCON_REDIS_EXPIRY)

finally:
api.redis_async = None
Expand All @@ -74,10 +65,10 @@ def test_post_vcon_expiry_value_is_3600(self):

@patch("api.add_vcon_to_set")
@patch("api.index_vcon_parties")
def test_post_vcon_with_ingress_list_stores_without_default_expiry(
def test_post_vcon_with_ingress_list_stores_with_default_expiry(
self, mock_index_vcon_parties, mock_add_vcon_to_set
):
"""Test that POST /vcon with ingress_lists stores and adds to list; no default vcon TTL."""
"""Test that POST /vcon with ingress_lists applies default TTL and queues it."""
mock_redis = MagicMock()
mock_json = MagicMock()
mock_json.set = AsyncMock()
Expand All @@ -95,13 +86,8 @@ def test_post_vcon_with_ingress_list_stores_without_default_expiry(
)

assert response.status_code == 201
# No default TTL on vcon key
vcon_key = f"vcon:{self.test_vcon['uuid']}"
vcon_expire_calls = [
c for c in mock_redis.expire.call_args_list
if c[0][0] == vcon_key and c[0][1] == VCON_REDIS_EXPIRY
]
assert len(vcon_expire_calls) == 0
mock_redis.expire.assert_awaited_once_with(vcon_key, VCON_REDIS_EXPIRY)
mock_redis.rpush.assert_called_once_with("test_ingress", self.test_vcon["uuid"])

finally:
Expand All @@ -121,10 +107,10 @@ def setup_method(self):
@patch("config.Configuration.get_ingress_auth")
@patch("api.add_vcon_to_set")
@patch("api.index_vcon_parties")
def test_external_ingress_stores_without_default_expiry(
def test_external_ingress_stores_with_default_expiry(
self, mock_index_vcon_parties, mock_add_vcon_to_set, mock_get_ingress_auth
):
"""Test that POST /vcon/external-ingress stores vCon; no default VCON_REDIS_EXPIRY on vcon key."""
"""Test that POST /vcon/external-ingress applies default TTL to the vCon."""
mock_get_ingress_auth.return_value = {self.ingress_list: self.valid_api_key}

mock_redis = MagicMock()
Expand All @@ -150,11 +136,7 @@ def test_external_ingress_stores_without_default_expiry(

assert response.status_code == 204
vcon_key = f"vcon:{self.test_vcon['uuid']}"
vcon_expire_calls = [
c for c in mock_redis.expire.call_args_list
if c[0][0] == vcon_key and c[0][1] == VCON_REDIS_EXPIRY
]
assert len(vcon_expire_calls) == 0, "vCon key should not get default VCON_REDIS_EXPIRY TTL"
mock_redis.expire.assert_awaited_once_with(vcon_key, VCON_REDIS_EXPIRY)

finally:
api.redis_async = None
Expand Down
Loading