diff --git a/core/ports.py b/core/ports.py index 090717b..879be0e 100644 --- a/core/ports.py +++ b/core/ports.py @@ -24,10 +24,6 @@ class RoomStorage(Protocol): """Save all data for a room.""" ... - def ensure_room(self, room_id: int) -> RoomData: - """Ensure a room exists, creating it if necessary. Returns RoomData.""" - ... - def add_bounty(self, room_id: int, bounty: Bounty) -> None: """Add a new bounty to a room. Creates room if it doesn't exist.""" ... @@ -50,7 +46,6 @@ class TrackingStorage(Protocol): """Storage port for tracking data. Tracks which bounties a user is tracking in a specific room. - Use ensure_tracking() to create a new tracking entry before tracking bounties. """ def load(self, room_id: int, user_id: int) -> TrackingData | None: @@ -61,12 +56,8 @@ class TrackingStorage(Protocol): """Save tracking data.""" ... - def ensure_tracking(self, room_id: int, user_id: int) -> TrackingData: - """Ensure tracking exists for user in room, creating if necessary. Returns TrackingData.""" - ... - def track_bounty(self, room_id: int, user_id: int, tracked: TrackedBounty) -> None: - """Add a bounty to a user's tracking list. Creates tracking if needed.""" + """Add a bounty to a user's tracking list. Creates tracking entry if needed.""" ... def untrack_bounty(self, room_id: int, user_id: int, bounty_id: int) -> None: diff --git a/tests/test_ports.py b/tests/test_ports.py index 65d40d2..3d67284 100644 --- a/tests/test_ports.py +++ b/tests/test_ports.py @@ -1,12 +1,87 @@ """Tests for core/ports.py — storage interfaces.""" import pytest -from typing import Self from core.models import Bounty, RoomData, TrackingData, TrackedBounty from core.ports import RoomStorage, TrackingStorage +class SimpleRoomStorage: + """Minimal mock without ensure_room - tests if add_bounty works without it. + + This mock only has the basic CRUD methods. It does NOT implement ensure_room(). + If add_bounty() still works with this simple mock, then ensure_room() may not + be needed as a public Protocol method. + """ + + def __init__(self): + self._rooms: dict[int, RoomData] = {} + + def load(self, room_id: int) -> RoomData | None: + return self._rooms.get(room_id) + + def save(self, room_data: RoomData) -> None: + self._rooms[room_data.room_id] = room_data + + def add_bounty(self, room_id: int, bounty: Bounty) -> None: + if room_id not in self._rooms: + self._rooms[room_id] = RoomData(room_id=room_id, bounties=[], next_id=1) + self._rooms[room_id].bounties.append(bounty) + + def update_bounty(self, room_id: int, bounty: Bounty) -> None: + if room_id in self._rooms: + for i, b in enumerate(self._rooms[room_id].bounties): + if b.id == bounty.id: + self._rooms[room_id].bounties[i] = bounty + break + + def delete_bounty(self, room_id: int, bounty_id: int) -> None: + if room_id in self._rooms: + self._rooms[room_id].bounties = [ + b for b in self._rooms[room_id].bounties if b.id != bounty_id + ] + + def get_bounty(self, room_id: int, bounty_id: int) -> Bounty | None: + if room_id in self._rooms: + for b in self._rooms[room_id].bounties: + if b.id == bounty_id: + return b + return None + + +class SimpleTrackingStorage: + """Minimal mock without ensure_tracking - tests if track_bounty works without it. + + This mock only has the basic methods. It does NOT implement ensure_tracking(). + If track_bounty() still works with this simple mock, then ensure_tracking() may not + be needed as a public Protocol method. + """ + + def __init__(self): + self._tracking: dict[tuple[int, int], TrackingData] = {} + + def load(self, room_id: int, user_id: int) -> TrackingData | None: + return self._tracking.get((room_id, user_id)) + + def save(self, tracking_data: TrackingData) -> None: + self._tracking[(tracking_data.room_id, tracking_data.user_id)] = tracking_data + + def track_bounty(self, room_id: int, user_id: int, tracked: TrackedBounty) -> None: + key = (room_id, user_id) + if key not in self._tracking: + self._tracking[key] = TrackingData( + room_id=room_id, user_id=user_id, tracked=[] + ) + self._tracking[key].tracked.append(tracked) + + def untrack_bounty(self, room_id: int, user_id: int, bounty_id: int) -> None: + key = (room_id, user_id) + if key in self._tracking: + self._tracking[key].tracked = [ + t for t in self._tracking[key].tracked if t.bounty_id != bounty_id + ] + + class MockRoomStorage: """Mock implementation of RoomStorage for testing.""" @@ -19,14 +94,9 @@ class MockRoomStorage: def save(self, room_data: RoomData) -> None: self._rooms[room_data.room_id] = room_data - def ensure_room(self, room_id: int) -> RoomData: - if room_id not in self._rooms: - self._rooms[room_id] = RoomData(room_id=room_id, bounties=[], next_id=1) - return self._rooms[room_id] - def add_bounty(self, room_id: int, bounty: Bounty) -> None: if room_id not in self._rooms: - self.ensure_room(room_id) + self._rooms[room_id] = RoomData(room_id=room_id, bounties=[], next_id=1) self._rooms[room_id].bounties.append(bounty) def update_bounty(self, room_id: int, bounty: Bounty) -> None: @@ -62,18 +132,12 @@ class MockTrackingStorage: def save(self, tracking_data: TrackingData) -> None: self._tracking[(tracking_data.room_id, tracking_data.user_id)] = tracking_data - def ensure_tracking(self, room_id: int, user_id: int) -> TrackingData: + def track_bounty(self, room_id: int, user_id: int, tracked: TrackedBounty) -> None: key = (room_id, user_id) if key not in self._tracking: self._tracking[key] = TrackingData( room_id=room_id, user_id=user_id, tracked=[] ) - return self._tracking[key] - - def track_bounty(self, room_id: int, user_id: int, tracked: TrackedBounty) -> None: - key = (room_id, user_id) - if key not in self._tracking: - self.ensure_tracking(room_id, user_id) self._tracking[key].tracked.append(tracked) def untrack_bounty(self, room_id: int, user_id: int, bounty_id: int) -> None: @@ -85,23 +149,32 @@ class MockTrackingStorage: class TestRoomStorage: + def test_simple_storage_without_ensure_room(self): + """Test that SimpleRoomStorage (no ensure_room) still works. + + This verifies that ensure_room() is NOT needed as a public Protocol method, + since add_bounty() can create the room internally without it. + """ + storage = SimpleRoomStorage() + bounty = Bounty( + id=1, + text="Test", + link=None, + due_date_ts=None, + created_at=0, + created_by_user_id=123, + ) + storage.add_bounty(-1001, bounty) + room = storage.load(-1001) + assert room is not None + assert len(room.bounties) == 1 + assert room.bounties[0].text == "Test" + assert room.next_id == 1 + def test_mock_implements_room_storage_protocol(self): storage: RoomStorage = MockRoomStorage() assert isinstance(storage, RoomStorage) - def test_ensure_room_creates_room_if_not_exists(self): - storage = MockRoomStorage() - room = storage.ensure_room(-1001) - assert room.room_id == -1001 - assert room.bounties == [] - assert room.next_id == 1 - - def test_ensure_room_returns_existing_room(self): - storage = MockRoomStorage() - room1 = storage.ensure_room(-1001) - room2 = storage.ensure_room(-1001) - assert room1 is room2 - def test_add_bounty_creates_room_if_not_exists(self): storage = MockRoomStorage() bounty = Bounty( @@ -158,23 +231,24 @@ class TestRoomStorage: class TestTrackingStorage: + def test_simple_storage_without_ensure_tracking(self): + """Test that SimpleTrackingStorage (no ensure_tracking) still works. + + This verifies that ensure_tracking() is NOT needed as a public Protocol method, + since track_bounty() can create the tracking entry internally without it. + """ + storage = SimpleTrackingStorage() + tracked = TrackedBounty(bounty_id=5, created_at=0) + storage.track_bounty(-1001, 123456, tracked) + result = storage.load(-1001, 123456) + assert result is not None + assert len(result.tracked) == 1 + assert result.tracked[0].bounty_id == 5 + def test_mock_implements_tracking_storage_protocol(self): storage: TrackingStorage = MockTrackingStorage() assert isinstance(storage, TrackingStorage) - def test_ensure_tracking_creates_if_not_exists(self): - storage = MockTrackingStorage() - tracking = storage.ensure_tracking(-1001, 123456) - assert tracking.room_id == -1001 - assert tracking.user_id == 123456 - assert tracking.tracked == [] - - def test_ensure_tracking_returns_existing(self): - storage = MockTrackingStorage() - t1 = storage.ensure_tracking(-1001, 123456) - t2 = storage.ensure_tracking(-1001, 123456) - assert t1 is t2 - def test_track_bounty_creates_tracking_if_not_exists(self): storage = MockTrackingStorage() tracked = TrackedBounty(bounty_id=5, created_at=0)