refactor: refactor http middleware #166
Closed
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
devhaozi
commented
Apr 9, 2025
Comment on lines
+110
to
+113
| func (r *Route) NotAllowed(handler contractshttp.HandlerFunc) { | ||
| panic("not support") | ||
| } | ||
|
|
There was a problem hiding this comment.
如 goravel/framework#996 (comment) 所提到，fiber 目前不支持自定义。
Comment on lines
+12
to
+31
| // nilHandler is a nil handler for global middleware to build chain. | ||
| var nilHandler contractshttp.HandlerFunc = func(ctx contractshttp.Context) error { | ||
| // TODO if use a fiber middleware as global middleware by type assert, they already call ctx.Next(), | ||
| // if duplicate call ctx.Next() will cause a error. Need to find a better way to handle this. | ||
| if ctx.Value("no_next") != nil { | ||
| // reset no_next flag for next global middleware | ||
| ctx.WithValue("no_next", nil) | ||
| return nil | ||
| } | ||
|
|
||
| fiberCtx := ctx.(*Context) | ||
| return fiberCtx.Instance().Next() | ||
| } | ||
|
|
||
| // invalidFiber instance.Context() will be nil when the request is timeout, | ||
| // the request will panic if ctx.Response() is called in this situation. | ||
| func invalidFiber(instance *fiber.Ctx) bool { | ||
| return instance.Context() == nil | ||
| } | ||
|
|
There was a problem hiding this comment.
这个空处理器是为了全局中间件构建处理器链（因为全局中间件没有路由处理器），但 fiber 有个特点是处理器需要调用Next来运行下一个处理器（全局中间件也一样，否则不会运行路由的处理器），所以就添加了fiberCtx.Instance().Next()调用。
这就导致一些比较 hacker 的用法（比如通过类型断言拿到 fiber 实例附加中间件）下，会重复调用Next，而 fiber 的设计如果调用Next时没有可用的处理器（所有处理器都用过了）的话会报错Cannot get xxx，这个用法框架内只在 cors 中间件用到，所以我添加了一个标志位来处理，如果有更好的优化方法可以评论讨论下。
1 task
devhaozi
commented
Apr 9, 2025
Comment on lines
+131
to
+137
| func (r *Group) getMiddlewares(handler httpcontract.HandlerFunc) fiber.Handler { | ||
| var middlewares []httpcontract.Middleware | ||
| middlewares = append(middlewares, r.originMiddlewares...) | ||
| middlewares = append(middlewares, r.middlewares...) | ||
| middlewares = append(middlewares, r.lastMiddlewares...) | ||
|
|
||
| return middlewares | ||
| return handlerToFiberHandler(middlewares, handler) |
There was a problem hiding this comment.
重写了获取中间件的方法，现在会先聚合全部中间件，然后传给handlerToFiberHandler函数构建处理器链
Comment on lines
+52
to
53
| assert.Equal(t, contractshttp.StatusText(contractshttp.StatusRequestTimeout), string(body)) | ||
| }) |
Comment on lines
+276
to
+285
| r.instance.Use(func(c *fiber.Ctx) error { | ||
| ctx := NewContext(c) | ||
| defer func() { | ||
| contextRequestPool.Put(ctx.request) | ||
| contextResponsePool.Put(ctx.response) | ||
| ctx.request = nil | ||
| ctx.response = nil | ||
| contextPool.Put(ctx) | ||
| }() | ||
| return r.fallback(ctx) |
There was a problem hiding this comment.
这里之前的实现 bug，没有回收 Context 导致资源泄露
Comment on lines
+294
to
+298
|
|
||
| // Interface guards | ||
| var ( | ||
| _ route.Route = (*Route)(nil) | ||
| ) |
There was a problem hiding this comment.
新增了 go 常用的接口守卫，确保 Route 实现了route.Route 的所有接口，防止运行时 panic。
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.
📑 Description
Closes goravel/goravel#655
✅ Checks