refactor: backend standardization (service layer, validation, route splitting) + image cache and Scryfall API fixes

This commit is contained in:
matt 2026-03-17 16:34:50 -07:00
parent e81b47bccf
commit f784741416
35 changed files with 7054 additions and 4344 deletions

View file

@ -96,7 +96,6 @@ def _write_dataset(path: Path) -> Path:
def _fresh_client(tmp_path: Path) -> tuple[TestClient, Path]:
dataset_path = _write_dataset(tmp_path / "partner_synergy.json")
os.environ["ENABLE_PARTNER_MECHANICS"] = "1"
os.environ["ENABLE_PARTNER_SUGGESTIONS"] = "1"
for module_name in (
"code.web.app",
"code.web.routes.partner_suggestions",
@ -177,7 +176,6 @@ def test_partner_suggestions_api_returns_ranked_candidates(tmp_path: Path) -> No
except Exception:
pass
os.environ.pop("ENABLE_PARTNER_MECHANICS", None)
os.environ.pop("ENABLE_PARTNER_SUGGESTIONS", None)
for module_name in (
"code.web.app",
"code.web.routes.partner_suggestions",
@ -245,7 +243,6 @@ def test_partner_suggestions_api_refresh_flag(monkeypatch) -> None:
from code.web.services.partner_suggestions import PartnerSuggestionResult
monkeypatch.setattr(route, "ENABLE_PARTNER_MECHANICS", True)
monkeypatch.setattr(route, "ENABLE_PARTNER_SUGGESTIONS", True)
captured: dict[str, bool] = {"refresh": False}

View file

@ -1,44 +0,0 @@
import base64
import json
from starlette.testclient import TestClient
def test_permalink_includes_locks_and_restores_notice(monkeypatch):
# Lazy import to ensure fresh app state
import importlib
app_module = importlib.import_module('code.web.app')
client = TestClient(app_module.app)
# Seed a session with a commander and locks by calling /build and directly touching session via cookie path
# Start a session
r = client.get('/build')
assert r.status_code == 200
# Now set some session state by invoking endpoints that mutate session
# Simulate selecting commander and a lock
# Use /build/from to load a permalink-like payload directly
payload = {
"commander": "Atraxa, Praetors' Voice",
"tags": ["proliferate"],
"bracket": 3,
"ideals": {"ramp": 10, "lands": 36, "basic_lands": 18, "creatures": 28, "removal": 10, "wipes": 3, "card_advantage": 8, "protection": 4},
"tag_mode": "AND",
"flags": {"owned_only": False, "prefer_owned": False},
"locks": ["Swords to Plowshares", "Sol Ring"],
}
raw = json.dumps(payload, separators=(",", ":")).encode('utf-8')
token = base64.urlsafe_b64encode(raw).decode('ascii').rstrip('=')
r2 = client.get(f'/build/from?state={token}')
assert r2.status_code == 200
# Step 4 should contain the locks restored chip
body = r2.text
assert 'locks restored' in body.lower()
# Ask the server for a permalink now and ensure locks are present
r3 = client.get('/build/permalink')
assert r3.status_code == 200
data = r3.json()
# Prefer decoded state when token not provided
state = data.get('state') or {}
assert 'locks' in state
assert set([s.lower() for s in state.get('locks', [])]) == {"swords to plowshares", "sol ring"}

View file

@ -0,0 +1,407 @@
"""Tests for service layer base classes, interfaces, and registry."""
from __future__ import annotations
import pytest
import time
from typing import Dict, Any
from code.web.services.base import (
BaseService,
StateService,
DataService,
CachedService,
ServiceError,
ValidationError,
NotFoundError,
)
from code.web.services.registry import ServiceRegistry, get_registry, reset_registry
from code.web.services.tasks import SessionManager
class TestBaseService:
"""Test BaseService abstract base class."""
def test_validation_helper(self):
"""Test _validate helper method."""
service = BaseService()
# Should not raise on True
service._validate(True, "Should not raise")
# Should raise on False
with pytest.raises(ValidationError, match="Should raise"):
service._validate(False, "Should raise")
class MockStateService(StateService):
"""Mock state service for testing."""
def _initialize_state(self, key: str) -> Dict[str, Any]:
return {"created": time.time(), "data": f"init-{key}"}
def _should_cleanup(self, key: str, state: Dict[str, Any]) -> bool:
# Cleanup if "expired" flag is set
return state.get("expired", False)
class TestStateService:
"""Test StateService base class."""
def test_get_state_creates_new(self):
"""Test that get_state creates new state."""
service = MockStateService()
state = service.get_state("test-key")
assert "created" in state
assert state["data"] == "init-test-key"
def test_get_state_returns_existing(self):
"""Test that get_state returns existing state."""
service = MockStateService()
state1 = service.get_state("test-key")
state1["custom"] = "value"
state2 = service.get_state("test-key")
assert state2 is state1
assert state2["custom"] == "value"
def test_set_and_get_value(self):
"""Test setting and getting state values."""
service = MockStateService()
service.set_state_value("key1", "field1", "value1")
assert service.get_state_value("key1", "field1") == "value1"
assert service.get_state_value("key1", "missing", "default") == "default"
def test_cleanup_state(self):
"""Test cleanup of expired state."""
service = MockStateService()
# Create some state
service.get_state("keep1")
service.get_state("keep2")
service.get_state("expire1")
service.get_state("expire2")
# Mark some as expired
service.set_state_value("expire1", "expired", True)
service.set_state_value("expire2", "expired", True)
# Cleanup
removed = service.cleanup_state()
assert removed == 2
# Verify expired are gone
state = service._state
assert "keep1" in state
assert "keep2" in state
assert "expire1" not in state
assert "expire2" not in state
class MockDataService(DataService[Dict[str, Any]]):
"""Mock data service for testing."""
def __init__(self, data: Dict[str, Any]):
super().__init__()
self._mock_data = data
def _load_data(self) -> Dict[str, Any]:
return self._mock_data.copy()
class TestDataService:
"""Test DataService base class."""
def test_lazy_loading(self):
"""Test that data is loaded lazily."""
service = MockDataService({"key": "value"})
assert not service.is_loaded()
data = service.get_data()
assert service.is_loaded()
assert data["key"] == "value"
def test_cached_loading(self):
"""Test that data is cached after first load."""
service = MockDataService({"key": "value"})
data1 = service.get_data()
data1["modified"] = True
data2 = service.get_data()
assert data2 is data1
assert data2["modified"]
def test_force_reload(self):
"""Test force reload of data."""
service = MockDataService({"key": "value"})
data1 = service.get_data()
data1["modified"] = True
data2 = service.get_data(force_reload=True)
assert data2 is not data1
assert "modified" not in data2
class MockCachedService(CachedService[str, int]):
"""Mock cached service for testing."""
def __init__(self, ttl_seconds: int | None = None, max_size: int | None = None):
super().__init__(ttl_seconds=ttl_seconds, max_size=max_size)
self.compute_count = 0
def _compute_value(self, key: str) -> int:
self.compute_count += 1
return len(key)
class TestCachedService:
"""Test CachedService base class."""
def test_cache_hit(self):
"""Test that values are cached."""
service = MockCachedService()
value1 = service.get("hello")
assert value1 == 5
assert service.compute_count == 1
value2 = service.get("hello")
assert value2 == 5
assert service.compute_count == 1 # Should not recompute
def test_cache_miss(self):
"""Test cache miss computes new value."""
service = MockCachedService()
value1 = service.get("hello")
value2 = service.get("world")
assert value1 == 5
assert value2 == 5
assert service.compute_count == 2
def test_ttl_expiration(self):
"""Test TTL-based expiration."""
service = MockCachedService(ttl_seconds=1)
value1 = service.get("hello")
assert service.compute_count == 1
# Should hit cache immediately
value2 = service.get("hello")
assert service.compute_count == 1
# Wait for expiration
time.sleep(1.1)
value3 = service.get("hello")
assert service.compute_count == 2 # Should recompute
def test_max_size_limit(self):
"""Test cache size limit."""
service = MockCachedService(max_size=2)
service.get("key1")
service.get("key2")
service.get("key3") # Should evict oldest (key1)
# key1 should be evicted
assert len(service._cache) == 2
assert "key1" not in service._cache
assert "key2" in service._cache
assert "key3" in service._cache
def test_invalidate_single(self):
"""Test invalidating single cache entry."""
service = MockCachedService()
service.get("key1")
service.get("key2")
service.invalidate("key1")
assert "key1" not in service._cache
assert "key2" in service._cache
def test_invalidate_all(self):
"""Test invalidating entire cache."""
service = MockCachedService()
service.get("key1")
service.get("key2")
service.invalidate()
assert len(service._cache) == 0
class MockService:
"""Mock service for registry testing."""
def __init__(self, value: str):
self.value = value
class TestServiceRegistry:
"""Test ServiceRegistry for dependency injection."""
def test_register_and_get_singleton(self):
"""Test registering and retrieving singleton."""
registry = ServiceRegistry()
instance = MockService("test")
registry.register_singleton(MockService, instance)
retrieved = registry.get(MockService)
assert retrieved is instance
assert retrieved.value == "test"
def test_register_and_get_factory(self):
"""Test registering and retrieving from factory."""
registry = ServiceRegistry()
registry.register_factory(MockService, lambda: MockService("factory"))
instance1 = registry.get(MockService)
instance2 = registry.get(MockService)
assert instance1 is not instance2 # Factory creates new instances
assert instance1.value == "factory"
assert instance2.value == "factory"
def test_lazy_singleton(self):
"""Test lazy-initialized singleton."""
registry = ServiceRegistry()
call_count = {"count": 0}
def factory():
call_count["count"] += 1
return MockService("lazy")
registry.register_lazy_singleton(MockService, factory)
instance1 = registry.get(MockService)
assert call_count["count"] == 1
instance2 = registry.get(MockService)
assert call_count["count"] == 1 # Should not call factory again
assert instance1 is instance2
def test_duplicate_registration_error(self):
"""Test error on duplicate registration."""
registry = ServiceRegistry()
registry.register_singleton(MockService, MockService("first"))
with pytest.raises(ValueError, match="already registered"):
registry.register_singleton(MockService, MockService("second"))
def test_get_unregistered_error(self):
"""Test error on getting unregistered service."""
registry = ServiceRegistry()
with pytest.raises(KeyError, match="not registered"):
registry.get(MockService)
def test_try_get(self):
"""Test try_get returns None for unregistered."""
registry = ServiceRegistry()
result = registry.try_get(MockService)
assert result is None
registry.register_singleton(MockService, MockService("test"))
result = registry.try_get(MockService)
assert result is not None
def test_is_registered(self):
"""Test checking if service is registered."""
registry = ServiceRegistry()
assert not registry.is_registered(MockService)
registry.register_singleton(MockService, MockService("test"))
assert registry.is_registered(MockService)
def test_unregister(self):
"""Test unregistering a service."""
registry = ServiceRegistry()
registry.register_singleton(MockService, MockService("test"))
assert registry.is_registered(MockService)
registry.unregister(MockService)
assert not registry.is_registered(MockService)
def test_clear(self):
"""Test clearing all services."""
registry = ServiceRegistry()
registry.register_singleton(MockService, MockService("test"))
registry.clear()
assert not registry.is_registered(MockService)
class TestSessionManager:
"""Test SessionManager refactored service."""
def test_new_session_id(self):
"""Test creating new session IDs."""
manager = SessionManager()
sid1 = manager.new_session_id()
sid2 = manager.new_session_id()
assert isinstance(sid1, str)
assert isinstance(sid2, str)
assert sid1 != sid2
assert len(sid1) == 32 # UUID hex is 32 chars
def test_get_session_creates_new(self):
"""Test get_session with None creates new."""
manager = SessionManager()
session = manager.get_session(None)
assert "created" in session
assert "updated" in session
def test_get_session_returns_existing(self):
"""Test get_session returns existing session."""
manager = SessionManager()
sid = manager.new_session_id()
session1 = manager.get_session(sid)
session1["custom"] = "data"
session2 = manager.get_session(sid)
assert session2 is session1
assert session2["custom"] == "data"
def test_set_and_get_value(self):
"""Test setting and getting session values."""
manager = SessionManager()
sid = manager.new_session_id()
manager.set_value(sid, "key1", "value1")
assert manager.get_value(sid, "key1") == "value1"
assert manager.get_value(sid, "missing", "default") == "default"
def test_cleanup_expired_sessions(self):
"""Test cleanup of expired sessions."""
manager = SessionManager(ttl_seconds=1)
sid1 = manager.new_session_id()
sid2 = manager.new_session_id()
manager.get_session(sid1)
time.sleep(1.1) # Let sid1 expire
manager.get_session(sid2) # sid2 is fresh
removed = manager.cleanup_state()
assert removed == 1
# sid1 should be gone, sid2 should exist
assert sid1 not in manager._state
assert sid2 in manager._state

View file

@ -0,0 +1,340 @@
"""Tests for validation framework (models, validators, card names)."""
from __future__ import annotations
import pytest
from pydantic import ValidationError
from code.web.validation.models import (
BuildRequest,
CommanderSearchRequest,
ThemeValidationRequest,
OwnedCardsImportRequest,
BatchBuildRequest,
CardReplacementRequest,
PowerBracket,
OwnedMode,
CommanderPartnerType,
)
from code.web.validation.card_names import CardNameValidator
from code.web.validation.validators import (
ThemeValidator,
PowerBracketValidator,
ColorIdentityValidator,
)
from code.web.validation.messages import ValidationMessages, MSG
class TestBuildRequest:
"""Test BuildRequest Pydantic model."""
def test_minimal_valid_request(self):
"""Test minimal valid build request."""
req = BuildRequest(commander="Atraxa, Praetors' Voice")
assert req.commander == "Atraxa, Praetors' Voice"
assert req.themes == []
assert req.power_bracket == PowerBracket.BRACKET_2
assert req.owned_mode == OwnedMode.OFF
def test_full_valid_request(self):
"""Test fully populated build request."""
req = BuildRequest(
commander="Kess, Dissident Mage",
themes=["Spellslinger", "Graveyard"],
power_bracket=PowerBracket.BRACKET_3,
owned_mode=OwnedMode.PREFER,
must_include=["Counterspell", "Lightning Bolt"],
must_exclude=["Armageddon"]
)
assert req.commander == "Kess, Dissident Mage"
assert len(req.themes) == 2
assert req.power_bracket == PowerBracket.BRACKET_3
assert len(req.must_include) == 2
def test_commander_whitespace_stripped(self):
"""Test commander name whitespace is stripped."""
req = BuildRequest(commander=" Atraxa ")
assert req.commander == "Atraxa"
def test_commander_empty_fails(self):
"""Test empty commander name fails validation."""
with pytest.raises(ValidationError):
BuildRequest(commander="")
with pytest.raises(ValidationError):
BuildRequest(commander=" ")
def test_themes_deduplicated(self):
"""Test themes are deduplicated case-insensitively."""
req = BuildRequest(
commander="Test",
themes=["Spellslinger", "spellslinger", "SPELLSLINGER", "Tokens"]
)
assert len(req.themes) == 2
assert "Spellslinger" in req.themes
assert "Tokens" in req.themes
def test_partner_validation_requires_name(self):
"""Test partner mode requires partner name."""
with pytest.raises(ValidationError, match="Partner mode requires partner_name"):
BuildRequest(
commander="Kydele, Chosen of Kruphix",
partner_mode=CommanderPartnerType.PARTNER
)
def test_partner_valid_with_name(self):
"""Test partner mode valid with name."""
req = BuildRequest(
commander="Kydele, Chosen of Kruphix",
partner_mode=CommanderPartnerType.PARTNER,
partner_name="Thrasios, Triton Hero"
)
assert req.partner_mode == CommanderPartnerType.PARTNER
assert req.partner_name == "Thrasios, Triton Hero"
def test_background_requires_name(self):
"""Test background mode requires background name."""
with pytest.raises(ValidationError, match="Background mode requires background_name"):
BuildRequest(
commander="Erinis, Gloom Stalker",
partner_mode=CommanderPartnerType.BACKGROUND
)
def test_custom_theme_requires_both(self):
"""Test custom theme requires both name and tags."""
with pytest.raises(ValidationError, match="Custom theme requires both name and tags"):
BuildRequest(
commander="Test",
custom_theme_name="My Theme"
)
with pytest.raises(ValidationError, match="Custom theme tags require theme name"):
BuildRequest(
commander="Test",
custom_theme_tags=["Tag1", "Tag2"]
)
class TestCommanderSearchRequest:
"""Test CommanderSearchRequest model."""
def test_valid_search(self):
"""Test valid search request."""
req = CommanderSearchRequest(query="Atraxa")
assert req.query == "Atraxa"
assert req.limit == 10
def test_custom_limit(self):
"""Test custom limit."""
req = CommanderSearchRequest(query="Test", limit=25)
assert req.limit == 25
def test_empty_query_fails(self):
"""Test empty query fails."""
with pytest.raises(ValidationError):
CommanderSearchRequest(query="")
def test_limit_bounds(self):
"""Test limit must be within bounds."""
with pytest.raises(ValidationError):
CommanderSearchRequest(query="Test", limit=0)
with pytest.raises(ValidationError):
CommanderSearchRequest(query="Test", limit=101)
class TestCardNameValidator:
"""Test card name validation and normalization."""
def test_normalize_lowercase(self):
"""Test normalization converts to lowercase."""
assert CardNameValidator.normalize("Atraxa, Praetors' Voice") == "atraxa, praetors' voice"
def test_normalize_removes_diacritics(self):
"""Test normalization removes diacritics."""
assert CardNameValidator.normalize("Dánitha Capashen") == "danitha capashen"
assert CardNameValidator.normalize("Gisela, the Broken Blade") == "gisela, the broken blade"
def test_normalize_standardizes_apostrophes(self):
"""Test normalization standardizes apostrophes."""
assert CardNameValidator.normalize("Atraxa, Praetors' Voice") == CardNameValidator.normalize("Atraxa, Praetors' Voice")
assert CardNameValidator.normalize("Atraxa, Praetors` Voice") == CardNameValidator.normalize("Atraxa, Praetors' Voice")
def test_normalize_collapses_whitespace(self):
"""Test normalization collapses whitespace."""
assert CardNameValidator.normalize("Test Card") == "test card"
assert CardNameValidator.normalize(" Test ") == "test"
def test_validator_caches_normalization(self):
"""Test validator caches normalized lookups."""
validator = CardNameValidator()
validator._card_names = {"Atraxa, Praetors' Voice"}
validator._normalized_map = {
"atraxa, praetors' voice": "Atraxa, Praetors' Voice"
}
validator._loaded = True
# Should find exact match
assert validator.is_valid("Atraxa, Praetors' Voice")
class TestThemeValidator:
"""Test theme validation."""
def test_validate_themes_separates_valid_invalid(self):
"""Test validation separates valid from invalid themes."""
validator = ThemeValidator()
validator._themes = {"Spellslinger", "spellslinger", "Tokens", "tokens"}
validator._loaded = True
valid, invalid = validator.validate_themes(["Spellslinger", "Invalid", "Tokens"])
assert "Spellslinger" in valid
assert "Tokens" in valid
assert "Invalid" in invalid
class TestPowerBracketValidator:
"""Test power bracket validation."""
def test_valid_brackets(self):
"""Test valid bracket values (1-4)."""
assert PowerBracketValidator.is_valid_bracket(1)
assert PowerBracketValidator.is_valid_bracket(2)
assert PowerBracketValidator.is_valid_bracket(3)
assert PowerBracketValidator.is_valid_bracket(4)
def test_invalid_brackets(self):
"""Test invalid bracket values."""
assert not PowerBracketValidator.is_valid_bracket(0)
assert not PowerBracketValidator.is_valid_bracket(5)
assert not PowerBracketValidator.is_valid_bracket(-1)
class TestColorIdentityValidator:
"""Test color identity validation."""
def test_parse_comma_separated(self):
"""Test parsing comma-separated colors."""
colors = ColorIdentityValidator.parse_colors("W,U,B")
assert colors == {"W", "U", "B"}
def test_parse_concatenated(self):
"""Test parsing concatenated colors."""
colors = ColorIdentityValidator.parse_colors("WUB")
assert colors == {"W", "U", "B"}
def test_parse_empty(self):
"""Test parsing empty string."""
colors = ColorIdentityValidator.parse_colors("")
assert colors == set()
def test_colorless_subset_any(self):
"""Test colorless cards valid in any deck."""
validator = ColorIdentityValidator()
assert validator.is_subset({"C"}, {"W", "U"})
assert validator.is_subset(set(), {"R", "G"})
def test_subset_validation(self):
"""Test subset validation."""
validator = ColorIdentityValidator()
# Valid: card colors subset of commander
assert validator.is_subset({"W", "U"}, {"W", "U", "B"})
# Invalid: card has colors not in commander
assert not validator.is_subset({"W", "U", "B"}, {"W", "U"})
class TestValidationMessages:
"""Test validation message formatting."""
def test_format_commander_invalid(self):
"""Test commander invalid message formatting."""
msg = MSG.format_commander_invalid("Test Commander")
assert "Test Commander" in msg
assert "not found" in msg
def test_format_themes_invalid(self):
"""Test multiple invalid themes formatting."""
msg = MSG.format_themes_invalid(["Theme1", "Theme2"])
assert "Theme1" in msg
assert "Theme2" in msg
def test_format_bracket_exceeded(self):
"""Test bracket exceeded message formatting."""
msg = MSG.format_bracket_exceeded("Mana Crypt", 4, 2)
assert "Mana Crypt" in msg
assert "4" in msg
assert "2" in msg
def test_format_color_mismatch(self):
"""Test color mismatch message formatting."""
msg = MSG.format_color_mismatch("Card", "WUB", "WU")
assert "Card" in msg
assert "WUB" in msg
assert "WU" in msg
class TestBatchBuildRequest:
"""Test batch build request validation."""
def test_valid_batch(self):
"""Test valid batch request."""
base = BuildRequest(commander="Test")
req = BatchBuildRequest(base_config=base, count=5)
assert req.count == 5
assert req.base_config.commander == "Test"
def test_count_limit(self):
"""Test batch count limit."""
base = BuildRequest(commander="Test")
with pytest.raises(ValidationError):
BatchBuildRequest(base_config=base, count=11)
class TestCardReplacementRequest:
"""Test card replacement request validation."""
def test_valid_replacement(self):
"""Test valid replacement request."""
req = CardReplacementRequest(card_name="Sol Ring", reason="Too powerful")
assert req.card_name == "Sol Ring"
assert req.reason == "Too powerful"
def test_whitespace_stripped(self):
"""Test whitespace is stripped."""
req = CardReplacementRequest(card_name=" Sol Ring ")
assert req.card_name == "Sol Ring"
def test_empty_name_fails(self):
"""Test empty card name fails."""
with pytest.raises(ValidationError):
CardReplacementRequest(card_name="")
class TestOwnedCardsImportRequest:
"""Test owned cards import validation."""
def test_valid_import(self):
"""Test valid import request."""
req = OwnedCardsImportRequest(format_type="csv", content="Name\nSol Ring\n")
assert req.format_type == "csv"
assert "Sol Ring" in req.content
def test_invalid_format(self):
"""Test invalid format fails."""
with pytest.raises(ValidationError):
OwnedCardsImportRequest(format_type="invalid", content="test")
def test_empty_content_fails(self):
"""Test empty content fails."""
with pytest.raises(ValidationError):
OwnedCardsImportRequest(format_type="csv", content="")