feat: remove chi dependency #8
Merged
Conversation
This file contains 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
WalkthroughThis pull request introduces significant changes to the httpsuite library, focusing on enhancing router support and modularizing the request parsing mechanism. The library now supports multiple routers (Gorilla MUX, Chi, and Go Standard) with a flexible parameter extraction approach. The project has been updated to version 2, with changes to the module structure, dependency management, and example implementations across different routing libraries. Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
examples/gorillamux/main.go (3)
23-33: Parameter setter pattern is flexible.Implementing
SetParamto populate individual fields from the URL parameter fosters reusability. Consider gracefully handling unknown fields to avoid silent failures.
35-37: Router-specific extractor.Using
mux.Vars(r)is a recognized approach with Gorilla Mux. This function is simple, but if additional logic is needed (e.g., type checking or boundary checks), consider adding it here.
45-66: Endpoint and server logic.
- The
ParseRequestusage withGorillaMuxParamExtractoris a good demonstration of decoupling router logic from request parsing.- Consider handling the error from
ListenAndServe(e.g.,log.Fatal(http.ListenAndServe(":8080", r))) to avoid silent failures.🧰 Tools
🪛 golangci-lint (1.62.2)
47-47: undefined: httpsuite
(typecheck)
60-60: undefined: httpsuite
(typecheck)
examples/stdmux/main.go (1)
45-71: HTTP server usage.
- Using
ParseRequestwithStdMuxParamExtractordemonstrates the router-agnostic design.- Consider handling errors from
http.ListenAndServeto make debugging easier.🧰 Tools
🪛 golangci-lint (1.62.2)
52-52: undefined: httpsuite
(typecheck)
65-65: undefined: httpsuite
(typecheck)
examples/chi/main.go (1)
24-34: Consider enhancing error handling in SetParam.The error message could be more descriptive to help diagnose issues.
- return err + return fmt.Errorf("failed to parse id '%s': %w", value, err)request_test.go (1)
38-48: Enhance MyParamExtractor implementation.The current implementation has limitations:
- Only supports "ID" parameter
- Assumes fixed URL structure
- No validation for malformed paths
func MyParamExtractor(r *http.Request, key string) string { pathSegments := strings.Split(r.URL.Path, "/") - // You should know how the path is structured; in this case, we expect the ID to be the second segment. - if len(pathSegments) > 2 && key == "ID" { - return pathSegments[2] + // Map path segments to parameter names + paramMap := map[string]int{ + "ID": 2, + // Add more parameters as needed + } + if pos, ok := paramMap[key]; ok && len(pathSegments) > pos { + return pathSegments[pos] } return "" }README.md (2)
26-28: Add language identifier to code block.The installation code block should specify the language for better syntax highlighting.
-``` +```bash go get github.com/rluders/httpsuite/v2<details> <summary>🧰 Tools</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> 26-26: Fenced code blocks should have a language specified null (MD040, fenced-code-language) </details> </details> --- `31-33`: **Consider adding quick start examples.** The Usage section could benefit from quick start examples for each supported router, showing the basic setup and request handling. I can help generate example code snippets for each supported router if you'd like. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 3363c157b42deacabd322b95ba93e623512dc380 and 7501c7e1dbff878126543f9b7973e07117373720. </details> <details> <summary>⛔ Files ignored due to path filters (4)</summary> * `examples/chi/go.sum` is excluded by `!**/*.sum` * `examples/gorillamux/go.sum` is excluded by `!**/*.sum` * `examples/stdmux/go.sum` is excluded by `!**/*.sum` * `go.sum` is excluded by `!**/*.sum` </details> <details> <summary>📒 Files selected for processing (11)</summary> * `README.md` (1 hunks) * `examples/chi/go.mod` (1 hunks) * `examples/chi/main.go` (1 hunks) * `examples/gorillamux/go.mod` (1 hunks) * `examples/gorillamux/main.go` (1 hunks) * `examples/main.go` (0 hunks) * `examples/stdmux/go.mod` (1 hunks) * `examples/stdmux/main.go` (1 hunks) * `go.mod` (1 hunks) * `request.go` (1 hunks) * `request_test.go` (7 hunks) </details> <details> <summary>💤 Files with no reviewable changes (1)</summary> * examples/main.go </details> <details> <summary>✅ Files skipped from review due to trivial changes (3)</summary> * examples/stdmux/go.mod * examples/gorillamux/go.mod * examples/chi/go.mod </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>README.md</summary> 26-26: Fenced code blocks should have a language specified null (MD040, fenced-code-language) </details> </details> <details> <summary>🪛 golangci-lint (1.62.2)</summary> <details> <summary>examples/chi/main.go</summary> 58-58: undefined: httpsuite (typecheck) --- 71-71: undefined: httpsuite (typecheck) </details> <details> <summary>examples/stdmux/main.go</summary> 52-52: undefined: httpsuite (typecheck) --- 65-65: undefined: httpsuite (typecheck) </details> <details> <summary>examples/gorillamux/main.go</summary> 47-47: undefined: httpsuite (typecheck) --- 60-60: undefined: httpsuite (typecheck) </details> </details> </details> <details> <summary>🔇 Additional comments (15)</summary><blockquote> <details> <summary>request.go (6)</summary> `18-28`: **Flexible URL parameter extraction looks good!** Introducing the `ParamExtractor` type is an elegant solution. It removes the tight coupling to a specific router (e.g., Chi) and allows more flexible usage with various routers. --- `30-59`: **Comprehensive documentation.** The enhanced doc comments thoroughly explain the usage of `ParseRequest`, including how the new `paramExtractor` function is leveraged. This is beneficial for both maintainers and new users alike. --- `62-66`: **Good practice closing request body.** Using `defer func() { _ = r.Body.Close() }()` ensures that the request body is properly closed, preventing resource leaks. The JSON decoding and error response logic is concise and correct. --- `76-78`: **Parameter extraction error handling is appropriate.** Returning a bad request status if a URL parameter is missing accurately communicates the nature of the error. This behavior is consistent with typical REST API expectations. --- `82-82`: **Useful internal server error response.** When `SetParam` fails, responding with a 500 makes sense, as this indicates an internal issue mapping the parameter. The `Details` field includes the actual error, which is helpful for debugging. --- `88-88`: **Validation error handling is consistent.** A structured validation error message with status 400 aligns with typical REST conventions, improving transparency for the client. </details> <details> <summary>examples/gorillamux/main.go (3)</summary> `11-15`: **Request struct is well-defined.** The `SampleRequest` with validation tags looks good. Ensure unit or integration tests exist to validate that requests missing the required fields trigger the appropriate error handling. --- `17-21`: **Response struct is straightforward.** Structuring the response as a separate entity is good practice, providing a clean separation of input (request) and output (response) data. --- `1-9`: **Dependency on local package reference.** The static analysis tool flagged `undefined: httpsuite`. Confirm that the import path (`github.com/rluders/httpsuite/v2`) is correct and available in your local environment or go.mod. </details> <details> <summary>examples/stdmux/main.go (4)</summary> `10-14`: **Well-structured request fields with validation tags.** Make sure to cover boundary cases in tests (e.g., missing or short values for `Name`, negative or zero values for `Age`). --- `16-20`: **Clear separation of response data.** A dedicated response struct is a clean approach, ensuring the server has a consistent output format. --- `22-32`: **Robust param setter.** The approach of converting `id` from string to int is standard. As a future improvement, consider validating the result to ensure it meets any domain-specific constraints (e.g., positive IDs). --- `1-8`: **Dependency on local package reference.** Similar to the Gorilla Mux file, ensure `github.com/rluders/httpsuite/v2` is properly set up in your modules. The static analysis warnings might be due to an incomplete local setup. </details> <details> <summary>examples/chi/main.go (1)</summary> `6-6`: **Update import path to use a specific version.** The import path `github.com/rluders/httpsuite/v2` should specify a version constraint to ensure reproducible builds. </details> <details> <summary>request_test.go (1)</summary> `107-107`: **LGTM! Test cases properly validate the new parameter extraction.** The test cases effectively cover the new parameter extraction functionality while maintaining existing behavior. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores