ajust config.Next position #1135
Conversation
middleware/cache/cache_test.go
Outdated
| }) | ||
|
|
||
| app.Get("/error", func(c *fiber.Ctx) error { | ||
| return c.Status(fiber.StatusInternalServerError).Send([]byte(time.Now().String())) |
There was a problem hiding this comment.
Why do you use Send here but not SendString?
There was a problem hiding this comment.
Oh, I will change that.
middleware/cache/cache_test.go
Outdated
| body, err := ioutil.ReadAll(resp.Body) | ||
| utils.AssertEqual(t, nil, err) | ||
|
|
||
| time.Sleep(500 * time.Millisecond) |
There was a problem hiding this comment.
What's the meaning of Sleep here?
There was a problem hiding this comment.
At the begining, I just want to make sure to return different string. Now I found it doesn't need to do that. I will fix it.
There was a problem hiding this comment.
Maybe we can take advantage of CacheControl option? If it is true, we'll get extra response header if it's cached.
There was a problem hiding this comment.
I add a new commit, plz have a look.
middleware/cache/cache_test.go
Outdated
| body, err = ioutil.ReadAll(resp.Body) | ||
| utils.AssertEqual(t, nil, err) | ||
|
|
||
| time.Sleep(500 * time.Millisecond) |
middleware/cache/cache_test.go
Outdated
|
|
||
| app.Use(New(Config{ | ||
| Next: func(c *fiber.Ctx) bool { | ||
| if c.Response().StatusCode() == fiber.StatusOK { |
There was a problem hiding this comment.
Please simplify these code 😃
tianjipeng
force-pushed
the
fix/cache
branch
2 times, most recently
from
05b8f52
to
daf5749
Compare
|
Please check error in CI |
|
Will the Custom Config readme example still be relevant with this change? https://github.com/gofiber/fiber/tree/master/middleware/cache#custom-config |
|
@kiyonlin I check the errorr, but it seems like proxy middleware test cases have data race: |
|
Re-running tests |
|
@kiyonlin I have removed body check, plz have a look. |
Please provide enough information so that others can review your pull request:
The intention of this PR is to address the issue raised here: 1116
Explain the details for making this change. What existing problem does the pull request solve?
Move
config.Nextafterctx.Next, so we can determine whether to cache or not after handling biz logic.Use emojis on commit messages so it provides an easy way of identifying the purpose or intention of a commit. Check out the emoji cheatsheet here: https://gitmoji.carloscuesta.me/