wrongmove/tests/unit/test_listing_cache.py
Viktor Barzin a42944a756 wrongmove: round-3 fix sweep — scrape pipeline, BUY tab, filter URL state, render hygiene, map polish
Coordinated fix across 31 bugs found in a parallel QA pass. Findings docs at /tmp/wrongmove-bugs/qa-round-3/qa{1,2,3,4}-*.md.

## Backend / scrape (Fix-1) — 8 bugs

- B1 [P0] Scrape totally broken on prod: pod UID 100 vs NFS dir 1000:1000 mode 775 → PermissionError on every never-seen listing. Switched Dockerfile to explicit `useradd --uid 1000 --gid 1000`; added securityContext + chown initContainer to k8s/{api,celery-beat}-deployment.yaml. Celery worker manifest lives outside this repo — Dockerfile UID change is the load-bearing fix.
- B4 [P1] Celery broker reaped every ~30s by Redis HAProxy idle timeout. Added `broker_transport_options` / `result_backend_transport_options` with `socket_keepalive=True, health_check_interval=25` in celery_app.py + same kwargs on every redis.from_url/Redis call across services/, utils/redis_lock.py, redis_repository.py.
- B5 [P1] dump_listings_task never published terminal FAILURE to the task_progress pub/sub channel — UI polled forever. Wrap body in try/except that publishes FAILURE before re-raising.
- B6 [P1] _process_worker had no per-listing exception handler — one bad listing killed the whole scrape via asyncio.gather. Wrap loop body in try/except Exception (re-raises CancelledError).
- B20 [P2] dump_listings_task gained time_limit=3600, soft_time_limit=3500, acks_late=True.
- B21 [P2] RedisRepository moved off shared db0 (was alongside paperless-ngx) to db3 via REDIS_USER_DB env var; keys prefixed `wrongmove:user:`.
- B32 [P3] redis_lock now uses uuid4() owner token + Lua compare-and-delete.
- B33 [P3] Slack notify in refresh_listings → asyncio.create_task (fire-and-forget).

## Frontend filter system (Fix-2) — 7 bugs

- B2 [P0] BUY tab click triggered "Maximum update depth exceeded" → ErrorBoundary. Replaced the three mutually-triggering useEffects in FilterBar with a single one-way controlled-value flow (URL → parent state → form), guarded by previousListingTypeRef so price-defaults fires once per real transition.
- B3 [P0] Filter values never reached the URL. Wired useFilterParams.setFilterValues into FilterBar/FilterPanel onSubmit + handleRemoveChip + new handleResetAllFilters; fed parsed filterValues into both forms' defaultValues; added URL→form sync via form.reset on browser back/forward.
- B8 [P1] Chip removal now resets form state via new FilterBar onFormReady callback — More badge no longer sticks.
- B12 [P2] Desktop swipe-review FAB added next to header (mobile FAB unchanged).
- B17 [P2] "Reset all" affordance on chip strip.
- B22 [P2] formatPrice precision: 1500 → £1.5k, 2500 → £2.5k (no longer collides with £2k/£3k defaults).
- B30 [P3] last_seen_days input gained min={0}.

## Frontend render hygiene + data integrity (Fix-3) — 8 bugs

- B7 [P1] streamingService bails on first non-NDJSON chunk (HTML response = backend down) and throws StreamParseError so the existing AlertError dialog surfaces a single user-visible error instead of 18× console.error spam.
- B9 [P1] formatDuration widened to (null|undefined|number): returns "—" for non-finite or negative, caps implausibly large values.
- B10 [P1] PropertyCard / PropertyCardCompact / SwipeCard JSX leaves render "—" for null total_price/qm/qmprice (was "£0/0 m²/£0/m²" — looked like free listings).
- B13 [P2] hexgrid worker reduceAverage uses Number.isFinite filter instead of !isNaN (which incorrectly accepted null → 0, biasing per-hex averages low).
- B14 [P2] ListingDetail Overview wraps agency in "Listed by" labelled block so it can't collapse to a bare agency name.
- B15 [P2] Compact POIDistanceBadges iterates all three travel modes with "—" for missing, matching the detail-sheet Travel table.
- B24 [P3] Drawer.Description (sr-only) added to ListingDetailSheet + MobileBottomSheet to silence Radix a11y warning.
- B25 [P3] lastSeenDays clamped to ≥0 so future timestamps don't render as "-7d ago".

## Frontend map / carousel / tasks polish (Fix-4) — 8 bugs

- B11 [P2] HexgridHeatmapClient destroy race: Map.tsx adds .catch() + ref guard so post-destroy promise rejections are silent no-ops. Verified by browser smoke (24 rapid Map↔List toggles → 0 pageErrors).
- B16 [P2] PhotoCarousel + inner CardCarousel gained keyboard nav (Arrow keys).
- B18 [P2] Default map center moved from Czech Republic to London (zoom 10).
- B19+B29 [P2/P3] Mapbox token: no longer hard-coded fallback; reads env-only and shows a clear "Map unavailable — set VITE_MAPBOX_TOKEN" banner when missing.
- B23 [P3] PhotoCarousel suppresses "1/1" counter for single-photo listings; added onError fallback for broken URLs.
- B26 [P3] PhotoCarousel only enables loop when photos.length > 1.
- B27 [P3] TaskIndicator cancel/clear-all buttons gained aria-label + data-testid.
- B28 [P3] useTaskProgress strips terminal-local task IDs from the polling union — no more forever-poll on completed tasks.

## Tests

74 new vitest tests + 18 new pytest tests. Local: tsc clean, 201 vitest tests pass, 633 pytest tests pass.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-10 22:27:29 +00:00

367 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."""
# Keepalive kwargs added to keep connection alive across the in-cluster
# Redis HAProxy 30s idle timeout. Same options are passed to every
# redis.from_url call in the codebase.
_KEEPALIVE_KWARGS = {
"decode_responses": True,
"socket_keepalive": True,
"health_check_interval": 25,
}
@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", **self._KEEPALIVE_KWARGS
)
@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", **self._KEEPALIVE_KWARGS
)
@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", **self._KEEPALIVE_KWARGS
)
@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", **self._KEEPALIVE_KWARGS
)
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