Chore: Optimize MongoDB Recipe #3332
Conversation
WalkthroughUpdated a literal greeting in an auth handler and refactored MongoDB CRUD handlers: simplified connection handling, standardized ObjectID hex usage, replaced manual cursor iteration with cursor.All, consolidated query/update/delete calls, and adjusted HTTP status handling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant API as HTTP Handler (mongodb/main.go)
participant DB as MongoDB
rect rgba(220,235,255,0.35)
note over API: DB connection: client -> Database(dbName)
API->>DB: Connect / select Database
DB-->>API: DB handle
end
alt List Employees
C->>API: GET /employees
API->>DB: collection.Find(bson.D{})
DB-->>API: Cursor
API->>API: cursor.All(...) \n defer cursor.Close
API-->>C: 200 JSON[]
else Get by ID
C->>API: GET /employees/:id
API->>API: Parse ObjectIDFromHex(id)
alt invalid id
API-->>C: 400
else valid id
API->>DB: collection.FindOne({_id: id})
alt not found
API-->>C: 404
else found
API-->>C: 200 JSON
end
end
else Insert
C->>API: POST /employees (JSON)
API->>DB: collection.InsertOne(doc)
DB-->>API: InsertedID
API->>API: emp.ID = InsertedID.Hex()
API-->>C: 201 JSON
else Update
C->>API: PUT /employees/:id (JSON)
API->>API: Parse ObjectIDFromHex(id)
alt invalid id
API-->>C: 400
else valid id
API->>DB: collection.FindOneAndUpdate({_id:id}, {$set: ...})
alt not found
API-->>C: 404
else updated
API-->>C: 200 JSON (with ID)
end
end
else Delete
C->>API: DELETE /employees/:id
API->>API: Parse ObjectIDFromHex(id)
alt invalid id
API-->>C: 400
else valid id
API->>DB: collection.DeleteOne({_id:id})
alt none deleted
API-->>C: 404
else deleted
API-->>C: 204
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Summary of ChangesHello @stewartmbofana, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the MongoDB recipe by optimizing its connection setup and refining the implementation of its CRUD API endpoints. The changes aim to make the code more concise, robust, and easier to maintain, while also addressing a small textual error in a separate authentication recipe. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several optimizations to the MongoDB recipe, making the code more idiomatic, efficient, and robust. The handlers for CRUD operations have been significantly simplified, and error handling is improved. A typo in another recipe was also fixed.
The changes are generally excellent. I've pointed out a critical issue regarding a resource leak due to a missing database disconnect call, which was inadvertently introduced while fixing another bug. I've also noted a medium-severity issue where an API response for empty data sets was changed from an empty array [] to null, which could be a breaking change for clients.
Overall, great work on cleaning up this recipe. Addressing these points will make it even better.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mongodb/main.go (1)
42-54: Critical: MongoDB connection is never closed.The removal of the deferred disconnect block creates a resource leak. Without calling
client.Disconnect(), MongoDB connections will accumulate and eventually exhaust the connection pool.Apply this diff to restore connection cleanup:
func Connect() error { client, err := mongo.Connect(context.TODO(), options.Client().ApplyURI(mongoURI)) if err != nil { return err } + // Verify connection + if err := client.Ping(context.TODO(), nil); err != nil { + return err + } + mg = MongoInstance{ Client: client, Db: client.Database(dbName), } return nil }Additionally, add a cleanup function and call it in main:
// Cleanup closes the MongoDB connection func Cleanup() error { return mg.Client.Disconnect(context.TODO()) }Then in
main(), add beforelog.Fatal:+ defer func() { + if err := Cleanup(); err != nil { + log.Println("Error disconnecting from MongoDB:", err) + } + }() + log.Fatal(app.Listen(":3000"))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
auth-docker-postgres-jwt/handler/api.go(1 hunks)mongodb/main.go(2 hunks)
🔇 Additional comments (5)
auth-docker-postgres-jwt/handler/api.go (1)
7-7: LGTM! Grammatical correction applied.The capitalization of "I" improves the user-facing message quality.
mongodb/main.go (4)
68-79: LGTM! Improved cursor handling.The addition of
defer cursor.Close(c.Context())ensures proper resource cleanup, and usingcursor.All()simplifies the code while maintaining correctness.
83-99: LGTM! Proper error handling and status codes.The error handling correctly distinguishes between invalid IDs (400), missing documents (404), and server errors (500), following REST conventions.
121-148: LGTM! Clean update implementation.The update operation correctly uses
FindOneAndUpdatewith proper error handling and status codes. Setting the ID from URL params ensures the response contains the correct identifier.
152-168: LGTM! Correct deletion logic and status codes.The delete operation properly returns 404 when the document doesn't exist and 204 (No Content) on successful deletion, following REST conventions.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mongodb/main.go (1)
34-38: Fix_iddecoding: string field now breaks all reads.MongoDB returns
_idasprimitive.ObjectID, butEmployee.IDis astring. With the newcursor.All/FindOnecalls, the driver will fail withcannot decode objectID into string, so both list and detail endpoints now return 500. Useprimitive.ObjectID(or a custom type) for the BSON field and convert to hex only when responding.type Employee struct { - ID string `json:"id,omitempty" bson:"_id,omitempty"` + ID primitive.ObjectID `json:"id,omitempty" bson:"_id,omitempty"` Name string `json:"name"` Salary float64 `json:"salary"` Age float64 `json:"age"` }Then, before returning JSON, assign
employee.ID = insertedIDand, if you need hex strings in responses, map to a DTO that exposesID: insertedID.Hex().
…bofana/recipes into feature/optimize-mongodb-recipe
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
mongodb/main.go (3)
43-55: Consider adding connection cleanup for production use.While simplified connection handling works for a recipe/example, production deployments should properly close the MongoDB connection to prevent resource leaks. Consider adding a cleanup mechanism or documenting this limitation.
Example cleanup approach:
// In main(), after app.Listen or use signal handling defer func() { if err := mg.Client.Disconnect(context.TODO()); err != nil { log.Printf("Error disconnecting from MongoDB: %v", err) } }()
75-75: Initialize as empty slice to avoid null in JSON response.When no employees exist, a nil slice serializes to
nullin JSON. Initialize as an empty slice for better API consistency.Apply this diff:
- var employees []Employee + employees := []Employee{}
143-152: Status code fix applied correctly; consider returning actual DB state.The HTTP 404 status code fix from the previous review has been properly applied. However, the handler returns the request body rather than fetching the updated document from the database. For a recipe, this simplification is acceptable, but production code should return the actual stored state to reflect any DB-side transformations or validations.
To return the actual updated document, you could use
FindOneAndUpdatewithoptions.After:opts := options.FindOneAndUpdate().SetReturnDocument(options.After) var updatedEmployee Employee err = mg.Db.Collection("employees").FindOneAndUpdate( c.Context(), bson.M{"_id": id}, update, opts, ).Decode(&updatedEmployee) if err != nil { if err == mongo.ErrNoDocuments { return c.SendStatus(http.StatusNotFound) } return c.SendStatus(http.StatusInternalServerError) } return c.JSON(updatedEmployee)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
mongodb/main.go(3 hunks)
🔇 Additional comments (4)
mongodb/main.go (4)
9-9: LGTM! Using HTTP status constants improves readability.The addition of
net/httpimport enables the use of named HTTP status constants (e.g.,http.StatusNotFound) throughout the handlers, which is more maintainable than numeric literals.
85-99: LGTM! Proper error handling and status codes.The handler correctly validates the ID format (400), distinguishes missing documents (404) from other errors (500), and returns the employee data directly. Well structured.
116-121: LGTM! Safe type assertion properly implemented.The safe type assertion from the previous review has been correctly implemented, preventing potential panics. Returning 201 with the created resource (including the generated ID) follows REST best practices.
168-172: LGTM! Correct status codes for delete operation.The HTTP 404 status code fix from the previous review has been properly applied. Returning 204 No Content on successful deletion is the correct REST pattern.
1 participant
Changes:
Summary by CodeRabbit
Style
Bug Fixes
Refactor