Add solution for gin challenge-2-middleware by Onkar-25 #1250
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughA new file introduces a complete Gin-based HTTP server with in-memory article storage, comprehensive middleware (request ID, logging, authentication, CORS, rate limiting, content-type validation), error handling, and CRUD endpoints for articles plus admin statistics. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.go`:
- Around line 418-446: In createArticle, avoid assigning IDs with
len(articles)+1 (which can collide after deletions); instead use a package-level
monotonic counter (e.g., nextID) and allocate the ID inside the write-protected
section: acquire the articles write lock, set newArticle.ID = nextID, increment
nextID, append to articles, then release the lock; update references to articles
and ensure nextID is declared and initialized once at package scope and always
modified under the same lock so IDs remain unique.
- Around line 448-485: The updateArticle handler currently replaces the stored
Article with the incoming payload, which lets clients change ID/timestamps and
drop created/updated timestamps; modify updateArticle to: parse and validate the
input via ShouldBindJSON into a temp Article (e.g., newArticle), use
findArticleByID(articleId) to get index and original article (old :=
articles[index]), reject if newArticle.ID != 0 && newArticle.ID != old.ID, copy
immutable fields from old into the updated record (ID, CreatedAt), update only
allowed mutable fields from newArticle (e.g., Title, Content), set UpdatedAt =
time.Now(), validate required fields on the merged record, then assign
articles[index] = mergedArticle and return it in the response. Ensure you
reference updateArticle, findArticleByID, Article, and articles when making the
changes.
- Around line 556-566: The function findArticleByID returns a pointer to the
loop variable (article) which is a copy; instead return a pointer to the actual
slice element so callers can mutate it. Modify the loop in findArticleByID to
iterate indices (or use index to reference the slice) and return
&articles[index] with the index; keep the same return semantics (pointer and
index, or nil and -1 when not found).
- Around line 238-257: RateLimitMiddleware's getLimiter currently creates
limiters with rate.NewLimiter(rate.Every(time.Minute), limit) which gives 1
token/min with burst 100; change the rate to
rate.Every(time.Minute/time.Duration(limit)) so the limiter refills at 100
req/min (burst still limit), and add logic to expire/cleanup entries in the
limiters map (e.g., track lastSeen per-IP and remove stale entries periodically)
to prevent unbounded growth of limiters.
- Around line 37-43: The global articles slice and nextID must be protected by a
sync.RWMutex to avoid races: add a package-level var (e.g., articlesMu
sync.RWMutex) and use articlesMu.RLock()/RUnlock() around all read-only accesses
(getArticles, getArticle, getStats, findArticleByID) and
articlesMu.Lock()/Unlock() around all mutations (createArticle, updateArticle,
deleteArticle); also update createArticle to allocate IDs from the shared nextID
(protected by the same mutex) instead of len(articles)+1 so concurrent creates
get unique IDs.
🧹 Nitpick comments (1)
packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.go (1)
517-543: Remove debug stdout logging from the stats handler.
fmt.Printlnis noisy in HTTP handlers; prefer structured logging with request ID (or drop it).🧹 Suggested cleanup
- fmt.Println("role",role)
Comment on lines
+37
to
+43
| // In-memory storage | ||
| var articles = []Article{ | ||
| {ID: 1, Title: "Getting Started with Go", Content: "Go is a programming language...", Author: "John Doe", CreatedAt: time.Now(), UpdatedAt: time.Now()}, | ||
| {ID: 2, Title: "Web Development with Gin", Content: "Gin is a web framework...", Author: "Jane Smith", CreatedAt: time.Now(), UpdatedAt: time.Now()}, | ||
| } | ||
| var nextID = 3 | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find packages/gin/challenge-2-middleware/submissions/Onkar-25 -name "*.go" | head -20Repository: RezaSi/go-interview-practice
Length of output: 140
🏁 Script executed:
wc -l packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.goRepository: RezaSi/go-interview-practice
Length of output: 144
🏁 Script executed:
head -100 packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.goRepository: RezaSi/go-interview-practice
Length of output: 3114
🏁 Script executed:
sed -n '100,250p' packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.goRepository: RezaSi/go-interview-practice
Length of output: 4103
🏁 Script executed:
sed -n '250,400p' packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.goRepository: RezaSi/go-interview-practice
Length of output: 3790
🏁 Script executed:
sed -n '400,584p' packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.goRepository: RezaSi/go-interview-practice
Length of output: 4636
Guard shared article state with a mutex to prevent data races.
The handlers (getArticles, getArticle, createArticle, updateArticle, deleteArticle, getStats) execute concurrently as HTTP handlers in Gin. They read and modify the global articles slice without synchronization, creating data races on:
articles = append(...)increateArticle(line 423) anddeleteArticle(line 486)articles[index] = newArticleinupdateArticle(line 459)- Concurrent reads in
getArticles(line 346) and viafindArticleByID()
Additionally, ID generation uses len(articles)+1 instead of nextID, which races when concurrent creates happen.
Add an RWMutex and lock around all reads/writes to the articles slice and nextID counter. Lock reads in getArticles, getArticle, getStats, and findArticleByID; lock writes in createArticle, updateArticle, and deleteArticle.
🔒 Suggested baseline for shared-state protection
-// In-memory storage
-var articles = []Article{
- {ID: 1, Title: "Getting Started with Go", Content: "Go is a programming language...", Author: "John Doe", CreatedAt: time.Now(), UpdatedAt: time.Now()},
- {ID: 2, Title: "Web Development with Gin", Content: "Gin is a web framework...", Author: "Jane Smith", CreatedAt: time.Now(), UpdatedAt: time.Now()},
-}
-var nextID = 3
+// In-memory storage
+var (
+ articlesMu sync.RWMutex
+ articles = []Article{
+ {ID: 1, Title: "Getting Started with Go", Content: "Go is a programming language...", Author: "John Doe", CreatedAt: time.Now(), UpdatedAt: time.Now()},
+ {ID: 2, Title: "Web Development with Gin", Content: "Gin is a web framework...", Author: "Jane Smith", CreatedAt: time.Now(), UpdatedAt: time.Now()},
+ }
+ nextID = 3
+)🤖 Prompt for AI Agents
In `@packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.go` around
lines 37 - 43, The global articles slice and nextID must be protected by a
sync.RWMutex to avoid races: add a package-level var (e.g., articlesMu
sync.RWMutex) and use articlesMu.RLock()/RUnlock() around all read-only accesses
(getArticles, getArticle, getStats, findArticleByID) and
articlesMu.Lock()/Unlock() around all mutations (createArticle, updateArticle,
deleteArticle); also update createArticle to allocate IDs from the shared nextID
(protected by the same mutex) instead of len(articles)+1 so concurrent creates
get unique IDs.
Comment on lines
+238
to
+257
| func RateLimitMiddleware() gin.HandlerFunc { | ||
| const limit = 100 | ||
|
|
||
| var ( | ||
| mu sync.Mutex | ||
| limiters = make(map[string]*rate.Limiter) | ||
| ) | ||
|
|
||
| getLimiter := func(ip string) *rate.Limiter { | ||
| mu.Lock() | ||
| defer mu.Unlock() | ||
|
|
||
| if l, ok := limiters[ip]; ok { | ||
| return l | ||
| } | ||
|
|
||
| // Effectively disables refill during the test loop | ||
| l := rate.NewLimiter(rate.Every(time.Minute), limit) | ||
| limiters[ip] = l | ||
| return l |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
golang.org/x/time/rate package NewLimiter rate.Every documentation
💡 Result:
From the golang.org/x/time/rate docs:
func Every(interval time.Duration) Limit: “Every converts a minimum time interval between events to aLimit.” For example,rate.Every(25*time.Millisecond)corresponds to 40 events/second. [1]func NewLimiter(r Limit, b int) *Limiter: “NewLimiter returns a new Limiter that allows events up to raterand permits bursts of at mostbtokens.” [1]
Example:
lim := rate.NewLimiter(rate.Every(25*time.Millisecond), 2) // ~40 req/s, burst up to 2Sources: [1] https://pkg.go.dev/github.com/golang/time/rate
Rate limiter refills at 1 req/min instead of 100 req/min.
rate.NewLimiter(rate.Every(time.Minute), limit) yields a 1/min refill rate with a burst capacity of 100 tokens. For sustained 100 req/min, use rate.Every(time.Minute/time.Duration(limit)). Additionally, consider expiring per-IP limiters to prevent unbounded map growth.
🐛 Fix refill rate to 100/min
- // Effectively disables refill during the test loop
- l := rate.NewLimiter(rate.Every(time.Minute), limit)
+ l := rate.NewLimiter(rate.Every(time.Minute/time.Duration(limit)), limit)📝 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.
Suggested change
| func RateLimitMiddleware() gin.HandlerFunc { | |
| const limit = 100 | |
| var ( | |
| mu sync.Mutex | |
| limiters = make(map[string]*rate.Limiter) | |
| ) | |
| getLimiter := func(ip string) *rate.Limiter { | |
| mu.Lock() | |
| defer mu.Unlock() | |
| if l, ok := limiters[ip]; ok { | |
| return l | |
| } | |
| // Effectively disables refill during the test loop | |
| l := rate.NewLimiter(rate.Every(time.Minute), limit) | |
| limiters[ip] = l | |
| return l | |
| func RateLimitMiddleware() gin.HandlerFunc { | |
| const limit = 100 | |
| var ( | |
| mu sync.Mutex | |
| limiters = make(map[string]*rate.Limiter) | |
| ) | |
| getLimiter := func(ip string) *rate.Limiter { | |
| mu.Lock() | |
| defer mu.Unlock() | |
| if l, ok := limiters[ip]; ok { | |
| return l | |
| } | |
| l := rate.NewLimiter(rate.Every(time.Minute/time.Duration(limit)), limit) | |
| limiters[ip] = l | |
| return l |
🤖 Prompt for AI Agents
In `@packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.go` around
lines 238 - 257, RateLimitMiddleware's getLimiter currently creates limiters
with rate.NewLimiter(rate.Every(time.Minute), limit) which gives 1 token/min
with burst 100; change the rate to rate.Every(time.Minute/time.Duration(limit))
so the limiter refills at 100 req/min (burst still limit), and add logic to
expire/cleanup entries in the limiters map (e.g., track lastSeen per-IP and
remove stale entries periodically) to prevent unbounded growth of limiters.
Comment on lines
+418
to
+446
| // createArticle handles POST /articles - create new article (protected) | ||
| func createArticle(c *gin.Context) { | ||
| // TODO: Parse JSON request body | ||
| var newArticle Article | ||
| if err:= c.ShouldBindJSON(&newArticle);err!=nil{ | ||
| c.JSON(400, APIResponse{ | ||
| Success: false, | ||
| Error: "Invalid user format", | ||
| }) | ||
| return | ||
| } | ||
| // TODO: Validate required fields | ||
| err := validateArticle(newArticle) | ||
| if err != nil{ | ||
| c.JSON(400, APIResponse{ | ||
| Success: false, | ||
| Error: "Invalid user format", | ||
| }) | ||
| return | ||
| } | ||
| // TODO: Add article to storage | ||
| newArticle.ID = len(articles)+1 | ||
| articles = append(articles,newArticle) | ||
| // TODO: Return created article | ||
| c.JSON(201, APIResponse{ | ||
| Success: true, | ||
| Data : newArticle, | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
ID assignment can collide after deletions.
Using len(articles)+1 will reuse IDs when a middle element is deleted. Use a monotonic counter (e.g., nextID) under the write lock.
🆔 Suggested fix for ID allocation
- newArticle.ID = len(articles)+1
- articles = append(articles,newArticle)
+ newArticle.ID = nextID
+ nextID++
+ articles = append(articles, newArticle)🤖 Prompt for AI Agents
In `@packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.go` around
lines 418 - 446, In createArticle, avoid assigning IDs with len(articles)+1
(which can collide after deletions); instead use a package-level monotonic
counter (e.g., nextID) and allocate the ID inside the write-protected section:
acquire the articles write lock, set newArticle.ID = nextID, increment nextID,
append to articles, then release the lock; update references to articles and
ensure nextID is declared and initialized once at package scope and always
modified under the same lock so IDs remain unique.
Comment on lines
+448
to
+485
| // updateArticle handles PUT /articles/:id - update article (protected) | ||
| func updateArticle(c *gin.Context) { | ||
| // TODO: Get article ID from URL parameter | ||
| id:= c.Param("id") | ||
| articleId ,err := strconv.Atoi(id) | ||
| if err != nil{ | ||
| c.JSON(http.StatusBadRequest, APIResponse{ | ||
| Success : false, | ||
| Error: "invalid id in input", | ||
| }) | ||
| return | ||
| } | ||
| // TODO: Find article by ID | ||
| _ , index := findArticleByID(articleId) | ||
| if index == -1{ | ||
| c.JSON(http.StatusNotFound, APIResponse{ | ||
| Success : false, | ||
| Error: "Aritcle Not Found", | ||
| }) | ||
| return | ||
| } | ||
| // TODO: Parse JSON request body | ||
| var newArticle Article | ||
| if err:= c.ShouldBindJSON(&newArticle);err!=nil{ | ||
| c.JSON(400, APIResponse{ | ||
| Success: false, | ||
| Error: "Invalid user format", | ||
| }) | ||
| return | ||
| } | ||
| // TODO: Find and update article | ||
| articles[index] = newArticle | ||
| // TODO: Return updated article | ||
| c.JSON(200, APIResponse{ | ||
| Success: true, | ||
| Data : articles[index], | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
Preserve ID/timestamps and validate on update.
The update handler overwrites stored data with the request payload, allowing ID changes and zeroing timestamps. Preserve immutable fields and re‑validate before saving.
✅ Suggested update flow
- var newArticle Article
- if err:= c.ShouldBindJSON(&newArticle);err!=nil{
+ var updated Article
+ if err:= c.ShouldBindJSON(&updated);err!=nil{
c.JSON(400, APIResponse{
Success: false,
Error: "Invalid user format",
})
return
}
- // TODO: Find and update article
- articles[index] = newArticle
+ updated.ID = articleId
+ updated.CreatedAt = articles[index].CreatedAt
+ updated.UpdatedAt = time.Now()
+ if err := validateArticle(updated); err != nil {
+ c.JSON(400, APIResponse{
+ Success: false,
+ Error: "Invalid user format",
+ })
+ return
+ }
+ // TODO: Find and update article
+ articles[index] = updated🤖 Prompt for AI Agents
In `@packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.go` around
lines 448 - 485, The updateArticle handler currently replaces the stored Article
with the incoming payload, which lets clients change ID/timestamps and drop
created/updated timestamps; modify updateArticle to: parse and validate the
input via ShouldBindJSON into a temp Article (e.g., newArticle), use
findArticleByID(articleId) to get index and original article (old :=
articles[index]), reject if newArticle.ID != 0 && newArticle.ID != old.ID, copy
immutable fields from old into the updated record (ID, CreatedAt), update only
allowed mutable fields from newArticle (e.g., Title, Content), set UpdatedAt =
time.Now(), validate required fields on the merged record, then assign
articles[index] = mergedArticle and return it in the response. Ensure you
reference updateArticle, findArticleByID, Article, and articles when making the
changes.
Comment on lines
+556
to
+566
| // findArticleByID finds an article by ID | ||
| func findArticleByID(id int) (*Article, int) { | ||
| // TODO: Implement article lookup | ||
| // Return article pointer and index, or nil and -1 if not found | ||
| for index,article := range articles{ | ||
| if article.ID == id{ | ||
| return &article,index | ||
| } | ||
| } | ||
| return nil, -1 | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and read the relevant section
cat -n packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.go | head -580 | tail -50Repository: RezaSi/go-interview-practice
Length of output: 1705
🏁 Script executed:
# Search for all usages of findArticleByID to understand how the returned pointer is used
rg "findArticleByID" packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.go -A 5 -B 2Repository: RezaSi/go-interview-practice
Length of output: 1016
🏁 Script executed:
# Also check if there are any modifications made to articles through the returned pointer
rg -A 10 "findArticleByID" packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.go | head -60Repository: RezaSi/go-interview-practice
Length of output: 1260
🏁 Script executed:
# Find the complete GET article handler to see if the article pointer is modified
rg -B 5 "article , index := findArticleByID" packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.go -A 15Repository: RezaSi/go-interview-practice
Length of output: 508
🏁 Script executed:
# Also check the Article struct to understand if it's a value or reference type
rg "type Article struct" packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.go -A 10Repository: RezaSi/go-interview-practice
Length of output: 398
🏁 Script executed:
# Check how articles slice is declared
rg "var articles" packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.go -B 2 -A 2Repository: RezaSi/go-interview-practice
Length of output: 424
Return a pointer to the slice element, not the range variable.
In Go, for index, article := range articles creates a copy of each element. Taking &article points to the loop variable (temporary copy), not the actual slice element in memory. The loop variable is reused in each iteration, making the pointer invalid. This breaks any in-place updates and violates Go pointer semantics.
🛠️ Correct pointer return
- for index,article := range articles{
- if article.ID == id{
- return &article,index
- }
- }
+ for index := range articles{
+ if articles[index].ID == id{
+ return &articles[index], index
+ }
+ }🤖 Prompt for AI Agents
In `@packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.go` around
lines 556 - 566, The function findArticleByID returns a pointer to the loop
variable (article) which is a copy; instead return a pointer to the actual slice
element so callers can mutate it. Modify the loop in findArticleByID to iterate
indices (or use index to reference the slice) and return &articles[index] with
the index; keep the same return semantics (pointer and index, or nil and -1 when
not found).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
gin challenge-2-middleware Solution
Submitted by: @Onkar-25
Package: gin
Challenge: challenge-2-middleware
Description
This PR contains my solution for gin challenge-2-middleware.
Changes
packages/gin/challenge-2-middleware/submissions/Onkar-25/solution.goTesting
Thank you for reviewing my submission! 🚀