- Increase Redis cache TTL from 30 minutes to 24 hours - Add stale-while-revalidate: serve stale cache (>4h) immediately while repopulating in background with SETNX lock to prevent concurrent rebuilds - Add in-memory frontend LRU cache (5 entries) so repeat filter visits are instant without network requests - Invalidate frontend cache on listing refresh and task completion - Add unit tests for get_cache_age, is_cache_stale, acquire_repopulation_lock
358 lines
13 KiB
Python
358 lines
13 KiB
Python
"""Unit tests for services/listing_cache.py."""
|
|
import json
|
|
from unittest import mock
|
|
|
|
import pytest
|
|
import redis
|
|
|
|
from models.listing import ListingType, QueryParameters
|
|
from services.listing_cache import (
|
|
CACHE_PREFIX,
|
|
CACHE_TTL_SECONDS,
|
|
STALE_AFTER_SECONDS,
|
|
_get_redis_client,
|
|
acquire_repopulation_lock,
|
|
cache_features_batch,
|
|
get_cache_age,
|
|
get_cached_count,
|
|
get_cached_features,
|
|
invalidate_cache,
|
|
is_cache_stale,
|
|
make_cache_key,
|
|
)
|
|
|
|
|
|
def _make_query(**overrides) -> QueryParameters:
|
|
"""Create a QueryParameters with defaults for testing."""
|
|
defaults = {"listing_type": ListingType.RENT, "min_price": 1000, "max_price": 3000}
|
|
defaults.update(overrides)
|
|
return QueryParameters(**defaults)
|
|
|
|
|
|
class TestMakeCacheKey:
|
|
"""Tests for make_cache_key()."""
|
|
|
|
def test_deterministic_for_same_params(self):
|
|
"""Same parameters produce the same cache key."""
|
|
qp = _make_query()
|
|
assert make_cache_key(qp) == make_cache_key(qp)
|
|
|
|
def test_different_for_different_params(self):
|
|
"""Different parameters produce different cache keys."""
|
|
qp1 = _make_query(min_price=1000)
|
|
qp2 = _make_query(min_price=2000)
|
|
assert make_cache_key(qp1) != make_cache_key(qp2)
|
|
|
|
def test_key_starts_with_prefix(self):
|
|
"""Cache key starts with CACHE_PREFIX."""
|
|
qp = _make_query()
|
|
assert make_cache_key(qp).startswith(CACHE_PREFIX)
|
|
|
|
|
|
class TestGetRedisClient:
|
|
"""Tests for _get_redis_client() URL parsing."""
|
|
|
|
@mock.patch("services.listing_cache.redis")
|
|
def test_default_broker_url(self, mock_redis):
|
|
"""Uses default localhost URL when env var is not set."""
|
|
with mock.patch.dict("os.environ", {}, clear=True):
|
|
_get_redis_client()
|
|
|
|
mock_redis.from_url.assert_called_once_with(
|
|
"redis://localhost:6379/2", decode_responses=True
|
|
)
|
|
|
|
@mock.patch("services.listing_cache.redis")
|
|
def test_custom_broker_url(self, mock_redis):
|
|
"""Replaces db number from custom broker URL."""
|
|
with mock.patch.dict(
|
|
"os.environ", {"CELERY_BROKER_URL": "redis://myhost:1234/5"}
|
|
):
|
|
_get_redis_client()
|
|
|
|
mock_redis.from_url.assert_called_once_with(
|
|
"redis://myhost:1234/2", decode_responses=True
|
|
)
|
|
|
|
@mock.patch("services.listing_cache.redis")
|
|
def test_broker_url_with_password(self, mock_redis):
|
|
"""Preserves auth info in broker URL."""
|
|
with mock.patch.dict(
|
|
"os.environ",
|
|
{"CELERY_BROKER_URL": "redis://:secret@myhost:6379/0"},
|
|
):
|
|
_get_redis_client()
|
|
|
|
mock_redis.from_url.assert_called_once_with(
|
|
"redis://:secret@myhost:6379/2", decode_responses=True
|
|
)
|
|
|
|
@mock.patch("services.listing_cache.redis")
|
|
def test_broker_url_with_query_params(self, mock_redis):
|
|
"""Preserves query parameters in broker URL."""
|
|
with mock.patch.dict(
|
|
"os.environ",
|
|
{"CELERY_BROKER_URL": "redis://myhost:6379/0?timeout=5"},
|
|
):
|
|
_get_redis_client()
|
|
|
|
mock_redis.from_url.assert_called_once_with(
|
|
"redis://myhost:6379/2?timeout=5", decode_responses=True
|
|
)
|
|
|
|
|
|
class TestGetCachedCount:
|
|
"""Tests for get_cached_count()."""
|
|
|
|
@mock.patch("services.listing_cache._get_redis_client")
|
|
def test_returns_none_on_redis_error(self, mock_get_client):
|
|
"""Returns None when Redis raises an error."""
|
|
mock_get_client.side_effect = redis.RedisError("connection refused")
|
|
|
|
result = get_cached_count(_make_query())
|
|
assert result is None
|
|
|
|
@mock.patch("services.listing_cache._get_redis_client")
|
|
def test_returns_none_when_key_not_exists(self, mock_get_client):
|
|
"""Returns None when the cache key does not exist."""
|
|
mock_client = mock.MagicMock()
|
|
mock_client.exists.return_value = False
|
|
mock_get_client.return_value = mock_client
|
|
|
|
result = get_cached_count(_make_query())
|
|
assert result is None
|
|
|
|
@mock.patch("services.listing_cache._get_redis_client")
|
|
def test_returns_count_when_key_exists(self, mock_get_client):
|
|
"""Returns list length when key exists."""
|
|
mock_client = mock.MagicMock()
|
|
mock_client.exists.return_value = True
|
|
mock_client.llen.return_value = 42
|
|
mock_get_client.return_value = mock_client
|
|
|
|
result = get_cached_count(_make_query())
|
|
assert result == 42
|
|
|
|
|
|
class TestGetCachedFeatures:
|
|
"""Tests for get_cached_features()."""
|
|
|
|
@mock.patch("services.listing_cache._get_redis_client")
|
|
def test_yields_empty_on_redis_error(self, mock_get_client):
|
|
"""Yields nothing when Redis raises an error."""
|
|
mock_get_client.side_effect = redis.RedisError("connection refused")
|
|
|
|
batches = list(get_cached_features(_make_query()))
|
|
assert batches == []
|
|
|
|
@mock.patch("services.listing_cache._get_redis_client")
|
|
def test_yields_batches(self, mock_get_client):
|
|
"""Yields features in batches."""
|
|
features = [{"type": "Feature", "id": i} for i in range(3)]
|
|
mock_client = mock.MagicMock()
|
|
mock_client.llen.return_value = 3
|
|
mock_client.lrange.return_value = [json.dumps(f) for f in features]
|
|
mock_get_client.return_value = mock_client
|
|
|
|
batches = list(get_cached_features(_make_query(), batch_size=50))
|
|
assert len(batches) == 1
|
|
assert batches[0] == features
|
|
|
|
|
|
class TestCacheFeaturesBatch:
|
|
"""Tests for cache_features_batch()."""
|
|
|
|
@mock.patch("services.listing_cache._get_redis_client")
|
|
def test_empty_features_returns_early(self, mock_get_client):
|
|
"""Does not call Redis when features list is empty."""
|
|
cache_features_batch(_make_query(), [])
|
|
mock_get_client.assert_not_called()
|
|
|
|
@mock.patch("services.listing_cache._get_redis_client")
|
|
def test_writes_features_via_pipeline(self, mock_get_client):
|
|
"""Writes features and sets TTL through pipeline."""
|
|
mock_client = mock.MagicMock()
|
|
mock_pipeline = mock.MagicMock()
|
|
mock_client.pipeline.return_value = mock_pipeline
|
|
mock_get_client.return_value = mock_client
|
|
|
|
features = [{"type": "Feature", "id": 1}]
|
|
cache_features_batch(_make_query(), features)
|
|
|
|
mock_pipeline.rpush.assert_called_once()
|
|
mock_pipeline.expire.assert_called_once()
|
|
mock_pipeline.execute.assert_called_once()
|
|
|
|
@mock.patch("services.listing_cache._get_redis_client")
|
|
def test_handles_redis_error(self, mock_get_client):
|
|
"""Handles Redis error gracefully during write."""
|
|
mock_get_client.side_effect = redis.RedisError("write error")
|
|
|
|
# Should not raise
|
|
cache_features_batch(_make_query(), [{"id": 1}])
|
|
|
|
|
|
class TestInvalidateCache:
|
|
"""Tests for invalidate_cache()."""
|
|
|
|
@mock.patch("services.listing_cache._get_redis_client")
|
|
def test_handles_redis_error(self, mock_get_client):
|
|
"""Handles Redis error gracefully during invalidation."""
|
|
mock_get_client.side_effect = redis.RedisError("connection refused")
|
|
|
|
# Should not raise
|
|
invalidate_cache()
|
|
|
|
@mock.patch("services.listing_cache._get_redis_client")
|
|
def test_deletes_matching_keys_via_pipeline(self, mock_get_client):
|
|
"""Deletes keys matching the cache prefix using pipeline."""
|
|
mock_client = mock.MagicMock()
|
|
mock_pipeline = mock.MagicMock()
|
|
mock_client.pipeline.return_value = mock_pipeline
|
|
# invalidate_cache scans two patterns (CACHE_PREFIX*, STAGING_PREFIX*)
|
|
# First scan returns matching keys, second returns none
|
|
mock_client.scan.side_effect = [
|
|
(0, ["listings:geojson:abc", "listings:geojson:def"]),
|
|
(0, []),
|
|
]
|
|
mock_get_client.return_value = mock_client
|
|
|
|
invalidate_cache()
|
|
|
|
assert mock_pipeline.delete.call_count == 2
|
|
mock_pipeline.execute.assert_called_once()
|
|
|
|
@mock.patch("services.listing_cache._get_redis_client")
|
|
def test_no_keys_to_delete(self, mock_get_client):
|
|
"""Does nothing when no cache keys exist."""
|
|
mock_client = mock.MagicMock()
|
|
mock_client.scan.return_value = (0, [])
|
|
mock_get_client.return_value = mock_client
|
|
|
|
invalidate_cache()
|
|
|
|
mock_client.pipeline.assert_not_called()
|
|
|
|
|
|
class TestCacheTTLConstants:
|
|
"""Tests for cache TTL constants."""
|
|
|
|
def test_cache_ttl_is_24_hours(self):
|
|
"""CACHE_TTL_SECONDS should be 24 hours."""
|
|
assert CACHE_TTL_SECONDS == 24 * 60 * 60
|
|
|
|
def test_stale_after_is_4_hours(self):
|
|
"""STALE_AFTER_SECONDS should be 4 hours."""
|
|
assert STALE_AFTER_SECONDS == 4 * 60 * 60
|
|
|
|
def test_stale_after_less_than_ttl(self):
|
|
"""Stale threshold must be less than the hard TTL."""
|
|
assert STALE_AFTER_SECONDS < CACHE_TTL_SECONDS
|
|
|
|
|
|
class TestGetCacheAge:
|
|
"""Tests for get_cache_age()."""
|
|
|
|
@mock.patch("services.listing_cache._get_redis_client")
|
|
def test_returns_none_when_key_missing(self, mock_get_client):
|
|
"""Returns None when key does not exist (ttl returns -2)."""
|
|
mock_client = mock.MagicMock()
|
|
mock_client.ttl.return_value = -2
|
|
mock_get_client.return_value = mock_client
|
|
|
|
result = get_cache_age(_make_query())
|
|
assert result is None
|
|
|
|
@mock.patch("services.listing_cache._get_redis_client")
|
|
def test_returns_none_when_no_expiry(self, mock_get_client):
|
|
"""Returns None when key has no TTL set (ttl returns -1)."""
|
|
mock_client = mock.MagicMock()
|
|
mock_client.ttl.return_value = -1
|
|
mock_get_client.return_value = mock_client
|
|
|
|
result = get_cache_age(_make_query())
|
|
assert result is None
|
|
|
|
@mock.patch("services.listing_cache._get_redis_client")
|
|
def test_computes_age_from_ttl(self, mock_get_client):
|
|
"""Age = CACHE_TTL_SECONDS - remaining TTL."""
|
|
mock_client = mock.MagicMock()
|
|
remaining = CACHE_TTL_SECONDS - 3600 # 1 hour old
|
|
mock_client.ttl.return_value = remaining
|
|
mock_get_client.return_value = mock_client
|
|
|
|
result = get_cache_age(_make_query())
|
|
assert result == 3600
|
|
|
|
@mock.patch("services.listing_cache._get_redis_client")
|
|
def test_returns_none_on_redis_error(self, mock_get_client):
|
|
"""Returns None when Redis raises an error."""
|
|
mock_get_client.side_effect = redis.RedisError("connection refused")
|
|
|
|
result = get_cache_age(_make_query())
|
|
assert result is None
|
|
|
|
|
|
class TestIsCacheStale:
|
|
"""Tests for is_cache_stale()."""
|
|
|
|
@mock.patch("services.listing_cache.get_cache_age")
|
|
def test_not_stale_when_young(self, mock_age):
|
|
"""Returns False when cache is younger than STALE_AFTER_SECONDS."""
|
|
mock_age.return_value = 100 # 100 seconds old
|
|
assert is_cache_stale(_make_query()) is False
|
|
|
|
@mock.patch("services.listing_cache.get_cache_age")
|
|
def test_stale_when_old(self, mock_age):
|
|
"""Returns True when cache is older than STALE_AFTER_SECONDS."""
|
|
mock_age.return_value = STALE_AFTER_SECONDS + 1
|
|
assert is_cache_stale(_make_query()) is True
|
|
|
|
@mock.patch("services.listing_cache.get_cache_age")
|
|
def test_not_stale_when_missing(self, mock_age):
|
|
"""Returns False when cache does not exist."""
|
|
mock_age.return_value = None
|
|
assert is_cache_stale(_make_query()) is False
|
|
|
|
@mock.patch("services.listing_cache.get_cache_age")
|
|
def test_not_stale_at_exact_threshold(self, mock_age):
|
|
"""Returns False when cache age equals STALE_AFTER_SECONDS exactly."""
|
|
mock_age.return_value = STALE_AFTER_SECONDS
|
|
assert is_cache_stale(_make_query()) is False
|
|
|
|
|
|
class TestAcquireRepopulationLock:
|
|
"""Tests for acquire_repopulation_lock()."""
|
|
|
|
@mock.patch("services.listing_cache._get_redis_client")
|
|
def test_acquires_lock_successfully(self, mock_get_client):
|
|
"""Returns True when lock is acquired (SETNX succeeds)."""
|
|
mock_client = mock.MagicMock()
|
|
mock_client.set.return_value = True
|
|
mock_get_client.return_value = mock_client
|
|
|
|
result = acquire_repopulation_lock(_make_query())
|
|
assert result is True
|
|
mock_client.set.assert_called_once()
|
|
# Verify nx=True and ex=60 were passed
|
|
call_kwargs = mock_client.set.call_args[1]
|
|
assert call_kwargs["nx"] is True
|
|
assert call_kwargs["ex"] == 60
|
|
|
|
@mock.patch("services.listing_cache._get_redis_client")
|
|
def test_returns_false_when_locked(self, mock_get_client):
|
|
"""Returns False when lock already held (SETNX returns None)."""
|
|
mock_client = mock.MagicMock()
|
|
mock_client.set.return_value = None
|
|
mock_get_client.return_value = mock_client
|
|
|
|
result = acquire_repopulation_lock(_make_query())
|
|
assert result is False
|
|
|
|
@mock.patch("services.listing_cache._get_redis_client")
|
|
def test_returns_false_on_redis_error(self, mock_get_client):
|
|
"""Returns False when Redis raises an error."""
|
|
mock_get_client.side_effect = redis.RedisError("connection refused")
|
|
|
|
result = acquire_repopulation_lock(_make_query())
|
|
assert result is False
|