feat(server/v2): Add Swagger UI support for server/v2 #23092
Conversation
crStiv
requested review from
julienrbrt,
hieuvubk,
kocubinski and
a team
as code owners
|
Warning Rate limit exceeded@crStiv has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 6 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe pull request introduces Swagger support for server/v2 by creating a new Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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
Documentation and Community
|
github-actions
bot
added
C:server/v2
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
server/v2/api/swagger/config.go(1 hunks)server/v2/api/swagger/handler.go(1 hunks)server/v2/server.go(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
server/v2/api/swagger/handler.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
server/v2/api/swagger/config.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
server/v2/server.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
🪛 golangci-lint (1.62.2)
server/v2/api/swagger/handler.go
44-44: undefined: time
(typecheck)
server/v2/api/swagger/config.go
24-24: undefined: fmt
(typecheck)
server/v2/server.go
78-78: undefined: http
(typecheck)
88-88: undefined: http
(typecheck)
251-251: s.config.API undefined (type ServerConfig has no field or method API)
(typecheck)
🔇 Additional comments (5)
server/v2/api/swagger/config.go (2)
5-11: Struct fields look good
Your Config struct is straightforward and clearly documents each field. This aligns well with Go style guidelines.
13-19: Clarity in default values
Returning Enable: false and Path: "/swagger" as defaults appears consistent with the code’s intended usage. This ensures that Swagger UI is turned off by default unless explicitly enabled.
server/v2/api/swagger/handler.go (1)
12-46: Handler logic and MIME handling
Your approach to handle file serving, MIME type detection, and fallback to index.html is solid. The usage of path.Clean is sensible for preventing tricky path manipulations. Once the time package is imported, http.ServeContent is a good fit for robust file serving.
🧰 Tools
🪛 golangci-lint (1.62.2)
44-44: undefined: time
(typecheck)
server/v2/server.go (2)
Line range hint 78-88: HTTP router usage
Defining router *http.ServeMux and assigning it with http.NewServeMux() is sensible for isolating routes. Confirm the design pattern for hooking in additional routes from other components if needed.
🧰 Tools
🪛 golangci-lint (1.62.2)
78-78: undefined: http
(typecheck)
249-263: Potential config mismatch
Line 251 references s.config.API.Swagger, but the static analysis suggests that ServerConfig has no field API. Verify your ServerConfig type to ensure it holds the API field or reference the correct nested structure.
🧰 Tools
🪛 golangci-lint (1.62.2)
251-251: s.config.API undefined (type ServerConfig has no field or method API)
(typecheck)
server/v2/api/swagger/config.go
Outdated
| // Validate validates the configuration | ||
| func (c Config) Validate() error { | ||
| if c.Path == "" { | ||
| return fmt.Errorf("swagger path cannot be empty") | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Ensure error formatting is enabled
While the validation logic is correct, remember that fmt.Errorf("swagger path cannot be empty") requires "fmt" to be imported (as noted above). Once the import is added, this code works as intended.
🧰 Tools
🪛 golangci-lint (1.62.2)
24-24: undefined: fmt
(typecheck)
server/v2/api/swagger/config.go
Outdated
| package swagger | ||
|
|
||
| import "github.com/cosmos/cosmos-sdk/server/v2/config" | ||
|
|
There was a problem hiding this comment.
Package import oversight
Currently, you're importing "github.com/cosmos/cosmos-sdk/server/v2/config" but using fmt.Errorf in the code without importing "fmt". There's no direct usage of the imported "github.com/cosmos/cosmos-sdk/server/v2/config" in this snippet. If it's needed in other parts of the file, that's fine. Otherwise, consider adding the missing "fmt" import or removing unused imports.
package swagger
-import "github.com/cosmos/cosmos-sdk/server/v2/config"
+import (
+ "fmt"
+ "github.com/cosmos/cosmos-sdk/server/v2/config"
+)
Committable suggestion skipped: line range outside the PR's diff.
server/v2/api/swagger/handler.go
Outdated
| import ( | ||
| "net/http" | ||
| "path" | ||
| "path/filepath" | ||
| "strings" | ||
|
|
||
| "github.com/rakyll/statik/fs" | ||
| ) |
There was a problem hiding this comment.
Missing time import
You're calling time.Time{} in this file but haven't imported "time". Add the missing import to avoid compilation issues.
import (
"net/http"
"path"
"path/filepath"
"strings"
"github.com/rakyll/statik/fs"
+ "time"
)Committable suggestion skipped: line range outside the PR's diff.
server/v2/server.go
Outdated
| "github.com/cosmos/cosmos-sdk/server/v2/api/swagger" | ||
| "github.com/pelletier/go-toml/v2" | ||
| "github.com/spf13/cobra" | ||
| "github.com/spf13/pflag" | ||
|
|
||
| "cosmossdk.io/core/transaction" | ||
| "cosmossdk.io/log" | ||
| "github.com/rakyll/statik/fs" |
There was a problem hiding this comment.
Check for missing imports
You reference swagger and statik/fs but also rely on http.ServeMux later (lines 78, 88) without importing "net/http". Ensure all required imports are present to avoid compilation errors.
import (
"context"
"errors"
"fmt"
"os"
"path/filepath"
"strings"
+ "net/http"
"github.com/cosmos/cosmos-sdk/server/v2/api/swagger"
"github.com/pelletier/go-toml/v2"
"github.com/spf13/cobra"Committable suggestion skipped: line range outside the PR's diff.
server/v2/server.go
Outdated
There was a problem hiding this comment.
Hi, thanks for tackling this. The wiring shouldn't be done in the main server, but api/swagger should be its own server component. Check out the other servers.
There was a problem hiding this comment.
Hi, thanks for tackling this. The wiring shouldn't be done in the main server, but api/swagger should be its own server component. Check out the other servers.
oh you're absolutely right, gonna fix it soon!
There was a problem hiding this comment.
@julienrbrt sorry for mess in commits, I had to redo some things but now everything should be fine
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
server/v2/api/swagger/config.go (1)
4-4: Remove unused imports.The import
"cosmossdk.io/core/server"is not needed and can be safely removed to adhere to Go’s best practices on unused imports.Here’s a suggested fix:
import ( - "cosmossdk.io/core/server" )🧰 Tools
🪛 golangci-lint (1.62.2)
4-4: "cosmossdk.io/core/server" imported and not used
(typecheck)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
server/v2/api/swagger/config.go(1 hunks)server/v2/api/swagger/server.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
server/v2/api/swagger/config.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
server/v2/api/swagger/server.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
🪛 golangci-lint (1.62.2)
server/v2/api/swagger/config.go
4-4: "cosmossdk.io/core/server" imported and not used
(typecheck)
server/v2/api/swagger/server.go
31-31: srv.Config undefined (type *Server[T] has no field or method Config, but does have field config)
(typecheck)
40-40: undefined: NewSwaggerHandler
(typecheck)
server/v2/api/swagger/server.go
Outdated
| srv.config = serverCfg | ||
|
|
||
| mux := http.NewServeMux() | ||
| mux.Handle(srv.config.Path, NewSwaggerHandler()) |
There was a problem hiding this comment.
Handle the undefined NewSwaggerHandler reference.
The function NewSwaggerHandler() is not defined in the reviewed code, nor is it imported from any package. Ensure that NewSwaggerHandler, or whichever handler function you’re calling, is properly defined and imported to prevent a runtime error or undefined symbol.
Would you like assistance creating a NewSwaggerHandler() function?
🧰 Tools
🪛 golangci-lint (1.62.2)
40-40: undefined: NewSwaggerHandler
(typecheck)
| cfgOptions: cfgOptions, | ||
| } | ||
|
|
||
| serverCfg := srv.Config().(*Config) |
There was a problem hiding this comment.
Fix the undefined method call.
Static analysis indicates that srv.Config() is undefined on line 31. The Server[T] struct does not define a Config() method; it only has a config field. Replace srv.Config().(*Config) with direct usage of the srv.config field.
Here’s a suggested fix:
- serverCfg := srv.Config().(*Config)
+ serverCfg := srv.config📝 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.
| serverCfg := srv.Config().(*Config) | |
| serverCfg := srv.config |
🧰 Tools
🪛 golangci-lint (1.62.2)
31-31: srv.Config undefined (type *Server[T] has no field or method Config, but does have field config)
(typecheck)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| // Open the file | ||
| file, err := statikFS.Open(urlPath) |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
|
|
||
| // Set caching headers | ||
| w.Header().Set("Cache-Control", "public, max-age=31536000") | ||
| w.Header().Set("Last-Modified", time.Now().UTC().Format(http.TimeFormat)) |
Check warning
Code scanning / CodeQL
Calling the system time Warning
| w.Header().Set("Last-Modified", time.Now().UTC().Format(http.TimeFormat)) | ||
|
|
||
| // Serve the file | ||
| http.ServeContent(w, r, urlPath, time.Now(), file.(io.ReadSeeker)) |
Check warning
Code scanning / CodeQL
Calling the system time Warning
julienrbrt
changed the title
Description
This PR adds Swagger UI support to the server/v2 package, providing an easy way to serve and interact with API documentation. The implementation includes a configurable handler for serving Swagger UI files and proper configuration options.
Closes: #23020
Key changes:
Author Checklist
I have...
[x] included the correct type prefix (feat) in the PR title
[x] confirmed ! in the type prefix if API or client breaking change (not needed)
[x] targeted the correct branch (main)
[x] provided a link to the relevant issue (#23020)
[x] reviewed "Files changed" and left comments if necessary
[x] included the necessary unit and integration tests
[x] added a changelog entry to CHANGELOG.md:
[x] updated the relevant documentation or specification
[x] confirmed all CI checks have passed
Reviewers Checklist
[ ] confirmed the correct type prefix in the PR title
[ ] confirmed all author checklist items have been addressed
[ ] reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage
Changed files:
Summary by CodeRabbit
New Features
Bug Fixes
Chores