Add Config.FileContent #1069
Add Config.FileContent #1069
Conversation
willfaught
requested review from
gaby,
sixcolors,
ReneWerner87 and
efectn
and removed request for
a team
WalkthroughThe update introduces a significant enhancement to the Swagger middleware, focusing on increased flexibility in handling Swagger specifications. It now allows the direct inclusion of Swagger file content within the configuration, reducing dependency on file paths. Additionally, comprehensive testing has been expanded to cover new functionalities, including custom paths and configurations, ensuring robust handling of multiple Swagger instances and custom routes. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- swagger/swagger.go (3 hunks)
- swagger/swagger_test.go (3 hunks)
Additional comments not posted (2)
swagger/swagger.go (1)
35-39: Consider documenting the expected format ofFileContent(JSON or YAML) for clarity.swagger/swagger_test.go (1)
4-4: Good use of theembedpackage to include swagger files directly in the test code.
| cfg := Config{FileContent: swaggerJSON} | ||
|
|
||
| app.Use(New(cfg)) | ||
|
|
||
| w1 := performRequest("GET", "/docs", app) | ||
| require.Equal(t, 200, w1.StatusCode) | ||
|
|
||
| w2 := performRequest("GET", "/swagger.json", app) | ||
| require.Equal(t, 200, w2.StatusCode) | ||
|
|
||
| w3 := performRequest("GET", "/notfound", app) | ||
| require.Equal(t, 404, w3.StatusCode) | ||
| }) | ||
|
|
||
| t.Run("Swagger file content not specified", func(t *testing.T) { | ||
| app := fiber.New() | ||
|
|
||
| cfg := Config{ | ||
| FilePath: "./docs/swagger.json", | ||
| } | ||
|
|
||
| require.Panics(t, func() { | ||
| app.Use(New(cfg)) | ||
| }, "content not specified") | ||
| }) | ||
|
|
||
| t.Run("Endpoint check with multiple Swagger instances", func(t *testing.T) { | ||
| app := fiber.New() | ||
|
|
||
| app.Use(New(Config{ | ||
| BasePath: "/api/v1", | ||
| FileContent: swaggerJSON, | ||
| })) | ||
|
|
||
| app.Use(New(Config{ | ||
| BasePath: "/api/v2", | ||
| FileContent: swaggerJSON, | ||
| })) | ||
|
|
||
| w1 := performRequest("GET", "/api/v1/docs", app) | ||
| require.Equal(t, 200, w1.StatusCode) | ||
|
|
||
| w2 := performRequest("GET", "/api/v1/swagger.json", app) | ||
| require.Equal(t, 200, w2.StatusCode) | ||
|
|
||
| w3 := performRequest("GET", "/api/v2/docs", app) | ||
| require.Equal(t, 200, w3.StatusCode) | ||
|
|
||
| w4 := performRequest("GET", "/api/v2/swagger.json", app) | ||
| require.Equal(t, 200, w4.StatusCode) | ||
|
|
||
| w5 := performRequest("GET", "/notfound", app) | ||
| require.Equal(t, 404, w5.StatusCode) | ||
| }) | ||
|
|
||
| t.Run("Endpoint check with custom routes", func(t *testing.T) { | ||
| app := fiber.New() | ||
|
|
||
| app.Use(New(Config{ | ||
| BasePath: "/api/v1", | ||
| FileContent: swaggerJSON, | ||
| })) | ||
|
|
||
| app.Get("/api/v1/tasks", func(c *fiber.Ctx) error { | ||
| return c.SendString("success") | ||
| }) | ||
|
|
||
| app.Get("/api/v1", func(c *fiber.Ctx) error { | ||
| return c.SendString("success") | ||
| }) | ||
|
|
||
| w1 := performRequest("GET", "/api/v1/docs", app) | ||
| require.Equal(t, 200, w1.StatusCode) | ||
|
|
||
| w2 := performRequest("GET", "/api/v1/swagger.json", app) | ||
| require.Equal(t, 200, w2.StatusCode) | ||
|
|
||
| w3 := performRequest("GET", "/notfound", app) | ||
| require.Equal(t, 404, w3.StatusCode) | ||
|
|
||
| // Verify we can send request to handler with the same BasePath as the middleware | ||
| w4 := performRequest("GET", "/api/v1/tasks", app) | ||
| bodyBytes, err := io.ReadAll(w4.Body) | ||
| require.NoError(t, err) | ||
| require.Equal(t, 200, w4.StatusCode) | ||
| require.Equal(t, "success", string(bodyBytes)) | ||
|
|
||
| // Verify handler in BasePath still works | ||
| w5 := performRequest("GET", "/api/v1", app) | ||
| bodyBytes, err = io.ReadAll(w5.Body) | ||
| require.NoError(t, err) | ||
| require.Equal(t, 200, w5.StatusCode) | ||
| require.Equal(t, "success", string(bodyBytes)) | ||
|
|
||
| w6 := performRequest("GET", "/api/v1/", app) | ||
| bodyBytes, err = io.ReadAll(w6.Body) | ||
| require.NoError(t, err) | ||
| require.Equal(t, 200, w6.StatusCode) | ||
| require.Equal(t, "success", string(bodyBytes)) | ||
| }) |
There was a problem hiding this comment.
Add negative test cases to ensure robust error handling, such as providing invalid JSON/YAML in FileContent.
Would you like me to help by adding these test cases?
|
Note that I tried to follow the way the existing code does and tests things. |
| if len(rawSpec) == 0 { | ||
| // Verify Swagger file exists | ||
| _, err := os.Stat(cfg.FilePath) | ||
| if os.IsNotExist(err) { |
There was a problem hiding this comment.
Use errors.Is(err, fs.ErrNotExist) instead
There was a problem hiding this comment.
That is existing code. I tried to minimize the changes. Do you still want me to change it?
|
@willfaught Want me to take over and get this merged? |
|
I forgot I had this PR open. If a maintainer is willing to attend to this PR to get it merged, I can reopen the PR. |
|
@willfaught I will take a look if you can re-open it, thanks! |
|
GitHub won't let me reopen this PR. Opened a new PR: #1085. |
I need to be able to embed a swagger.json file into a Go program using the embed package. Config.FileContent enables that.
Summary by CodeRabbit