shoko/jigaido
1
0
Fork 0
Code Issues 3 Pull Requests Actions Packages Projects Releases Wiki Activity

Add core/ports.py - Storage interfaces #20

Merged
shoko merged 3 commits from feat/issue-6-storage-ports into main 2026-04-03 08:58:30 +02:00
Conversation 8 Commits 3 Files Changed 3 +339
shoko commented 2026-04-03 00:40:26 +02:00
Owner
Copy Link

Summary

Define abstract storage interfaces (Protocols) for storage adapters:

  • RoomStorage: for room/group bounties - load, save, add_bounty, update_bounty, delete_bounty, get_bounty
  • PersonalStorage: same operations for personal/DM bounties stored in RoomData
  • TrackingStorage: for tracking data - load, save, track_bounty, untrack_bounty

Changes

  • core/ports.py - New file with Protocol definitions
  • core/__init__.py - Updated exports to include storage protocols

Closes #6

## Summary Define abstract storage interfaces (Protocols) for storage adapters: - **RoomStorage**: for room/group bounties - load, save, add_bounty, update_bounty, delete_bounty, get_bounty - **PersonalStorage**: same operations for personal/DM bounties stored in RoomData - **TrackingStorage**: for tracking data - load, save, track_bounty, untrack_bounty ## Changes - `core/ports.py` - New file with Protocol definitions - `core/__init__.py` - Updated exports to include storage protocols Closes #6
shoko added 1 commit 2026-04-03 00:40:26 +02:00
Add core/ports.py - Storage interfaces 5450d12400 
Define abstract storage interfaces (Protocols):
- RoomStorage: for room/group bounties (load, save, add/update/delete/get_bounty)
- PersonalStorage: same operations for personal/DM bounties
- TrackingStorage: for tracking data (load, save, track/untrack_bounty)
han commented 2026-04-03 01:53:48 +02:00
Collaborator
Copy Link

why do we need personal storage when we now doesnt really separate which one is personal and which one is group? I think its somehow already covered in the tracking storage and room storage.

what is storage / protocol anyway? we add bounty through room storage, but how to add a new room/tracking?

why do we need personal storage when we now doesnt really separate which one is personal and which one is group? I think its somehow already covered in the tracking storage and room storage. what is storage / protocol anyway? we add bounty through room storage, but how to add a new room/tracking?
han commented 2026-04-03 01:54:26 +02:00
Collaborator
Copy Link

is it possible to create unit tests in this PR?

is it possible to create unit tests in this PR?
shoko referenced this issue from a commit 2026-04-03 01:57:57 +02:00
Address PR #20 feedback:
shoko added 1 commit 2026-04-03 01:57:57 +02:00
Address PR #20 feedback: 43603659de 
- Removed PersonalStorage (redundant - RoomStorage handles both via room_id)
- Added ensure_room() and ensure_tracking() methods for explicit creation
- Added @runtime_checkable to Protocols for isinstance checks
- Added tests/test_ports.py with 11 unit tests for storage protocols
shoko commented 2026-04-03 01:58:39 +02:00
Author
Owner
Copy Link

Addressed all feedback:

1. PersonalStorage removed

You are right - PersonalStorage was redundant. RoomStorage handles both group and personal bounties via room_id:

  • Negative room_id = Telegram group (e.g., -1001)
  • Positive room_id = DM/personal context (user's Telegram ID)

2. Creating rooms/tracking entries

Added explicit ensure_room() and ensure_tracking() methods:

  • RoomStorage.ensure_room(room_id) - creates room if it doesn't exist
  • TrackingStorage.ensure_tracking(room_id, user_id) - creates tracking entry if it doesn't exist

The add_bounty and track_bounty methods also create the room/tracking implicitly if needed.

3. Unit tests added

Added tests/test_ports.py with 11 tests covering protocol compliance, creation methods, and CRUD operations.

Test results: 22 passed, 1 warning

Addressed all feedback: **1. PersonalStorage removed** You are right - PersonalStorage was redundant. RoomStorage handles both group and personal bounties via room_id: - Negative room_id = Telegram group (e.g., -1001) - Positive room_id = DM/personal context (user's Telegram ID) **2. Creating rooms/tracking entries** Added explicit `ensure_room()` and `ensure_tracking()` methods: - `RoomStorage.ensure_room(room_id)` - creates room if it doesn't exist - `TrackingStorage.ensure_tracking(room_id, user_id)` - creates tracking entry if it doesn't exist The `add_bounty` and `track_bounty` methods also create the room/tracking implicitly if needed. **3. Unit tests added** Added `tests/test_ports.py` with 11 tests covering protocol compliance, creation methods, and CRUD operations. Test results: 22 passed, 1 warning
han reviewed 2026-04-03 08:43:03 +02:00
tests/test_ports.py
@@ -0,0 +19,4 @@
def save(self, room_data: RoomData) -> None:
self._rooms[room_data.room_id] = room_data
def ensure_room(self, room_id: int) -> RoomData:
han commented 2026-04-03 08:43:03 +02:00
Collaborator
Copy Link

can we add a test where we have a very simple mock storage to check or verify if we really need an ensure_room function? basically the goal is to see do we need the ensure_room() or we can safely remove it and expect things also works without it.

can we add a test where we have a very simple mock storage to check or verify if we really need an ensure_room function? basically the goal is to see do we need the ensure_room() or we can safely remove it and expect things also works without it.
shoko added 1 commit 2026-04-03 08:49:01 +02:00
Remove ensure_room/ensure_tracking from Protocol - tests prove not needed a237810dd2 
Tests with SimpleRoomStorage and SimpleTrackingStorage (without ensure_*)
show that add_bounty() and track_bounty() work fine without explicit
ensure methods - they create rooms/tracking internally.

This simplifies the Protocol to only essential methods.
shoko commented 2026-04-03 08:49:24 +02:00
Author
Owner
Copy Link

Added tests with SimpleRoomStorage and SimpleTrackingStorage (no ensure_* methods):

Test Result: 20 passed, 1 warning

Key findings:

  • SimpleRoomStorage (without ensure_room()) - add_bounty works fine
  • SimpleTrackingStorage (without ensure_tracking()) - track_bounty works fine

Conclusion: ensure_room() and ensure_tracking() are NOT needed as public Protocol methods.

The add_bounty() and track_bounty() methods create rooms/tracking internally. Removed these methods from the Protocol to simplify the interface.

Added tests with SimpleRoomStorage and SimpleTrackingStorage (no ensure_* methods): **Test Result: 20 passed, 1 warning** Key findings: - `SimpleRoomStorage` (without `ensure_room()`) - add_bounty works fine - `SimpleTrackingStorage` (without `ensure_tracking()`) - track_bounty works fine **Conclusion: `ensure_room()` and `ensure_tracking()` are NOT needed as public Protocol methods.** The add_bounty() and track_bounty() methods create rooms/tracking internally. Removed these methods from the Protocol to simplify the interface.
han commented 2026-04-03 08:52:44 +02:00
Collaborator
Copy Link

Conclusion: ensure_room() and ensure_tracking() are NOT needed as public Protocol methods.

I think we should remove ensure_room() and ensure_tracking() then since its adding unnecessary function as well. what do you think?

> Conclusion: ensure_room() and ensure_tracking() are NOT needed as public Protocol methods. I think we should remove ensure_room() and ensure_tracking() then since its adding unnecessary function as well. what do you think?
shoko commented 2026-04-03 08:54:38 +02:00
Author
Owner
Copy Link

Confirmed - ensure_room() and ensure_tracking() have already been removed from the Protocol definitions in the previous commit.

Current Protocols:

RoomStorage: load, save, add_bounty, update_bounty, delete_bounty, get_bounty

TrackingStorage: load, save, track_bounty, untrack_bounty

Test Results: 20 passed, 1 warning

Confirmed - ensure_room() and ensure_tracking() have already been removed from the Protocol definitions in the previous commit. **Current Protocols:** **RoomStorage:** load, save, add_bounty, update_bounty, delete_bounty, get_bounty **TrackingStorage:** load, save, track_bounty, untrack_bounty **Test Results: 20 passed, 1 warning**
han approved these changes 2026-04-03 08:57:48 +02:00
han left a comment
Collaborator
Copy Link

lgtm

lgtm
shoko merged commit 8aebb763ee into main 2026-04-03 08:58:30 +02:00
Sign in to join this conversation.
Reviewers
No Reviewers
han
Labels
Clear labels
adapter
Storage adapter
 architecture
Architecture/design
 cleanup
Cleanup task
 cli
CLI application
 core
Core domain layer
 enhancement
need-user-approval
phase-1
Phase 1 task
 phase-2
Phase 2 implementation task
 telegram-bot
Telegram bot app
 tests
Tests
 v2
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
han shoko
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: shoko/jigaido#20
Delete Branch "feat/issue-6-storage-ports"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?