-
Notifications
You must be signed in to change notification settings - Fork 0
First official version of the chess rust api using rest architecture #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe codebase undergoes a major architectural transformation, replacing a custom chess library with an Axum-based REST API service. The old modular design (Casa, Jogador, Peca traits, Posicao, Tabuleiro) is removed entirely, and game logic now delegates to the shakmaty chess library via a new in-memory game store with UUID-based game sessions and RESTful endpoints. Changes
Sequence DiagramsequenceDiagram
participant Client
participant API as API Server
participant Store as Game Store
participant Engine as shakmaty Engine
Client->>API: POST /games (optional FEN)
API->>Engine: Parse FEN or use default
Engine-->>API: Position created
API->>Store: Create GameEntry (UUID)
Store-->>API: Store game state
API-->>Client: GameResponse { id, fen }
Client->>API: POST /games/:id/moves (UCI move)
API->>Store: Get GameEntry by id
Store-->>API: Current Position + History
API->>Engine: Parse UCI move string
Engine->>Engine: Validate move legality
Engine-->>API: Move object
API->>Engine: Apply move to position
Engine-->>API: New position + status
API->>Engine: Generate SAN from move
Engine-->>API: SAN notation
API->>Store: Update position & history
Store-->>API: Success
API-->>Client: GameResponse { fen, status, legal_moves, applied_move }
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~55 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main.rs`:
- Around line 22-34: The in-memory Store (type Store, AppState.store holding
GameEntry) is unbounded and can exhaust memory; add capacity and eviction
controls by replacing or wrapping Store with a bounded structure (e.g., an LRU
or size-limited HashMap) and track last_access/created timestamps on GameEntry
(add a timestamp field to GameEntry) so stale games can be expired; enforce a
max game count in the game-creation code path (check AppState.max_games before
inserting) and implement a background cleanup task that removes entries older
than AppState.ttl, and optionally add simple rate-limiting checks on the
create-game endpoint to prevent rapid creation from one client. Ensure reads
update the GameEntry timestamp so TTL reflects inactivity and use atomic
counters/locks in AppState to make max-count checks race-safe.
🧹 Nitpick comments (3)
src/main.rs (3)
136-136:CorsLayer::permissive()allows all origins.This is overly permissive for production environments, allowing any website to make requests to your API. For development/first version this is acceptable, but before production deployment, configure specific allowed origins.
139-139: Consider making the bind address configurable.The port is hardcoded to 3000. Consider reading from an environment variable for deployment flexibility:
♻️ Suggested improvement
- let addr: SocketAddr = ([0, 0, 0, 0], 3000).into(); + let port: u16 = std::env::var("PORT") + .ok() + .and_then(|p| p.parse().ok()) + .unwrap_or(3000); + let addr: SocketAddr = ([0, 0, 0, 0], port).into();
322-326: Remove unnecessary comment.The comment about "Using a trick" is misleading since
pos.turn()is the straightforward, documented way to get the side to move.♻️ Suggested cleanup
fn side_to_move(pos: &Chess) -> Color { - // Using a trick: if any legal move exists for White when pos is white to move, but we can directly access via pos.turn() - // Shakmaty exposes the side to move via pos.turn(). pos.turn() }
| type Store = Arc<RwLock<HashMap<Uuid, GameEntry>>>; | ||
|
|
||
| #[derive(Clone)] | ||
| struct AppState { | ||
| store: Store, | ||
| } | ||
|
|
||
| #[derive(Debug)] | ||
| struct GameEntry { | ||
| pos: Chess, | ||
| history_uci: Vec<String>, | ||
| history_san: Vec<String>, | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unbounded in-memory store could lead to memory exhaustion.
The store has no size limits. A malicious actor could create unlimited games, exhausting server memory. Consider adding:
- Maximum game count limit
- TTL/expiration for inactive games
- Rate limiting on game creation
This is acceptable for an initial version but should be addressed before production deployment.
🤖 Prompt for AI Agents
In `@src/main.rs` around lines 22 - 34, The in-memory Store (type Store,
AppState.store holding GameEntry) is unbounded and can exhaust memory; add
capacity and eviction controls by replacing or wrapping Store with a bounded
structure (e.g., an LRU or size-limited HashMap) and track last_access/created
timestamps on GameEntry (add a timestamp field to GameEntry) so stale games can
be expired; enforce a max game count in the game-creation code path (check
AppState.max_games before inserting) and implement a background cleanup task
that removes entries older than AppState.ttl, and optionally add simple
rate-limiting checks on the create-game endpoint to prevent rapid creation from
one client. Ensure reads update the GameEntry timestamp so TTL reflects
inactivity and use atomic counters/locks in AppState to make max-count checks
race-safe.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.