Add solution for gin challenge-1-basic-routing by yllmis #922
Conversation
WalkthroughThis PR introduces a complete Gin-based HTTP API for in-memory user management with CRUD operations and search functionality. Defines User and Response types, initializes storage with seed data, and implements six endpoint handlers plus helper functions for validation and lookup. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
Possibly related PRs
Pre-merge checks✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/gin/challenge-1-basic-routing/submissions/yllmis/solution.go (5)
68-68: Improve message text for grammatical correctness.The message "Successfully" is grammatically incomplete. Consider "Success" or "Retrieved successfully".
102-102: Improve message text for grammatical correctness.The message "Successfully" is grammatically incomplete. Consider "Success" or "Retrieved successfully".
232-232: Fix grammatical error in message text.The message should be "Successfully deleted" or "Deleted successfully" (not "Successfully delete").
253-253: Use English for code comments.Lines 253 and 261 contain Chinese comments. For consistency and broader readability in open-source projects, use English.
Also applies to: 261-261
281-290: Email format validation not implemented.The TODO comment mentions validating email format, but only presence is checked. While basic validation may suffice for this challenge, consider adding email format validation using a regex or the
net/mailpackage if stricter validation is required.🔎 Example using net/mail for email validation
+import ( + "net/mail" +) + func validateUser(user User) error { if user.Name == "" || user.Email == "" { return errors.New("name and email are required") } + if _, err := mail.ParseAddress(user.Email); err != nil { + return errors.New("invalid email format") + } return nil }
| var users = []User{ | ||
| {ID: 1, Name: "John Doe", Email: "[email protected]", Age: 30}, | ||
| {ID: 2, Name: "Jane Smith", Email: "[email protected]", Age: 25}, | ||
| {ID: 3, Name: "Bob Wilson", Email: "[email protected]", Age: 35}, | ||
| } | ||
| var nextID = 4 |
There was a problem hiding this comment.
Critical: Data race on shared mutable state.
The users slice and nextID variable are accessed and modified concurrently by multiple HTTP handlers without any synchronization (mutex, channels, etc.). This creates data races that can lead to lost updates, corrupted data, or panics.
🔎 Recommended fix using sync.Mutex
+import (
+ "sync"
+)
+
// In-memory storage
+var mu sync.Mutex
var users = []User{
{ID: 1, Name: "John Doe", Email: "[email protected]", Age: 30},
{ID: 2, Name: "Jane Smith", Email: "[email protected]", Age: 25},
{ID: 3, Name: "Bob Wilson", Email: "[email protected]", Age: 35},
}
var nextID = 4Then wrap all reads/writes to users and nextID with mu.Lock() and mu.Unlock() in each handler.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/gin/challenge-1-basic-routing/submissions/yllmis/solution.go around
lines 31 to 36 the global users slice and nextID variable are shared mutable
state accessed by HTTP handlers and cause data races; add a package-level mutex
(preferably sync.RWMutex) and wrap all accesses: use mu.Lock()/mu.Unlock() for
any writes (adding, updating, deleting users or incrementing nextID) and
mu.RLock()/mu.RUnlock() for read-only handlers to protect the slice and nextID,
ensuring every handler acquires the appropriate lock before accessing or
mutating those globals.
| func main() { | ||
| // TODO: Create Gin router | ||
| r := gin.Default() | ||
|
|
||
| // TODO: Setup routes | ||
| // GET /users - Get all users | ||
| r.GET("/users", getAllUsers) | ||
| // GET /users/:id - Get user by ID | ||
| r.GET("/users/:id", getUserByID) | ||
| // POST /users - Create new user | ||
| r.POST("/users", createUser) | ||
| // PUT /users/:id - Update user | ||
| r.PUT("/users/:id", updateUser) | ||
| // DELETE /users/:id - Delete user | ||
| r.DELETE("/users/:id", deleteUser) | ||
| // GET /users/search - Search users by name | ||
| r.GET("/users/search", searchUsers) | ||
|
|
||
| // TODO: Start server on port 8080 | ||
| r.Run(":8080") | ||
| } |
There was a problem hiding this comment.
Critical: Route ordering prevents search endpoint from being reachable.
The route /users/search (line 54) is registered after /users/:id (line 46). In Gin, routes are matched in registration order, so a request to /users/search will match the :id parameter route first, treating "search" as an ID. The searchUsers handler will never execute.
🔎 Fix: Register specific routes before parameterized routes
// TODO: Setup routes
// GET /users - Get all users
r.GET("/users", getAllUsers)
+ // GET /users/search - Search users by name (must be before :id route)
+ r.GET("/users/search", searchUsers)
// GET /users/:id - Get user by ID
r.GET("/users/:id", getUserByID)
// POST /users - Create new user
r.POST("/users", createUser)
// PUT /users/:id - Update user
r.PUT("/users/:id", updateUser)
// DELETE /users/:id - Delete user
r.DELETE("/users/:id", deleteUser)
- // GET /users/search - Search users by name
- r.GET("/users/search", searchUsers)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func main() { | |
| // TODO: Create Gin router | |
| r := gin.Default() | |
| // TODO: Setup routes | |
| // GET /users - Get all users | |
| r.GET("/users", getAllUsers) | |
| // GET /users/:id - Get user by ID | |
| r.GET("/users/:id", getUserByID) | |
| // POST /users - Create new user | |
| r.POST("/users", createUser) | |
| // PUT /users/:id - Update user | |
| r.PUT("/users/:id", updateUser) | |
| // DELETE /users/:id - Delete user | |
| r.DELETE("/users/:id", deleteUser) | |
| // GET /users/search - Search users by name | |
| r.GET("/users/search", searchUsers) | |
| // TODO: Start server on port 8080 | |
| r.Run(":8080") | |
| } | |
| func main() { | |
| // TODO: Create Gin router | |
| r := gin.Default() | |
| // TODO: Setup routes | |
| // GET /users - Get all users | |
| r.GET("/users", getAllUsers) | |
| // GET /users/search - Search users by name | |
| r.GET("/users/search", searchUsers) | |
| // GET /users/:id - Get user by ID | |
| r.GET("/users/:id", getUserByID) | |
| // POST /users - Create new user | |
| r.POST("/users", createUser) | |
| // PUT /users/:id - Update user | |
| r.PUT("/users/:id", updateUser) | |
| // DELETE /users/:id - Delete user | |
| r.DELETE("/users/:id", deleteUser) | |
| // TODO: Start server on port 8080 | |
| r.Run(":8080") | |
| } |
🤖 Prompt for AI Agents
In packages/gin/challenge-1-basic-routing/submissions/yllmis/solution.go around
lines 38 to 58, the specific route GET /users/search is registered after the
parameterized route GET /users/:id so requests to /users/search will be captured
as an id; fix it by registering the more specific route(s) (e.g.,
r.GET("/users/search", searchUsers)) before any parameterized route(s) like
r.GET("/users/:id", getUserByID) so the search handler is reachable.
1 participant
gin challenge-1-basic-routing Solution
Submitted by: @yllmis
Package: gin
Challenge: challenge-1-basic-routing
Description
This PR contains my solution for gin challenge-1-basic-routing.
Changes
packages/gin/challenge-1-basic-routing/submissions/yllmis/solution.goTesting
Thank you for reviewing my submission! 🚀