Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(hz): optimize the naming style for middleware #660

Merged
merged 6 commits into from
Apr 10, 2023

Conversation

FGYFFFF
Copy link
Contributor

@FGYFFFF FGYFFFF commented Mar 10, 2023

What type of PR is this?

feat

Check the PR title.

  • This PR title match the format: <type>(optional scope): <description>
  • The description of this PR title is user-oriented and clear enough for others to understand.

(Optional) Translate the PR title into Chinese.

优化中间件的命名方式

(Optional) More detail description for this PR(en: English/zh: Chinese).

en: optimize the naming style for middleware
zh(optional): 优化中间件的命名方式
修改内容:

  • 严格区分"组中间件"和"handler中间件"的命名

形如 "a/b" & "a/b/c" 这种路由的定义,生成的代码如下:

root := r.Group("/", rootMw()...)
{
    _a := root.Group("/a", _aMw()...)
    _a.GET("/b", append(_bMw(), hertz.Method1)...)
    _b := _a.Group("/b", _bMw()...)
    _b.POST("/c", append(_method2Mw(), hertz.Method2)...)
}

可以看到_a.GET("/b", append(_bMw(), hertz.Method1)...)_b := _a.Group("/b", _bMw()...)共用了同一个 middleware 函数。产生问题的原因是没有严格区分 "group middleware" 和 "handler middleware" 的命名方式,导致生产的代码共用了同一个中间件函数。

预期生成的代码为:

root := r.Group("/", rootMw()...)
{
    _a := root.Group("/a", _aMw()...)
    _a.GET("/b", append(_method1Mw(), hertz.Method1)...)
    _b := _a.Group("/b", _bMw()...)
    _b.POST("/c", append(_method2Mw(), hertz.Method2)...)
}

是否有break:
如果有类似的路由注册,会多生成一个中间件,需要用户本身注意下是否需要增加修改中间件的内容

  • 增加中间件命名风格:
    增加以 "path" 作为分隔的中间件命名分隔,例如"a_b_mws()";从而不需要对中间件的命名进行 "Unique" 操作,进而对齐内部的命名方式

  • 修复了一个命名转换的bug,会错误地将 "A"->"_"

Which issue(s) this PR fixes:

@FGYFFFF FGYFFFF requested review from a team as code owners March 10, 2023 09:27
@FGYFFFF FGYFFFF changed the title fix: handler middleware name fix(hz): handler middleware name Mar 10, 2023
@codecov
Copy link

codecov bot commented Mar 10, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (ab37d29) 75.64% compared to head (29207a8) 75.64%.

❗ Current head 29207a8 differs from pull request most recent head 2a27d78. Consider uploading reports for the commit 2a27d78 to get more accurate results

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #660   +/-   ##
========================================
  Coverage    75.64%   75.64%           
========================================
  Files           97       97           
  Lines         9462     9462           
========================================
  Hits          7158     7158           
  Misses        1855     1855           
  Partials       449      449           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@FGYFFFF FGYFFFF force-pushed the fix/handler_middleware_name branch 2 times, most recently from e858bdf to 1c87f8d Compare March 10, 2023 10:13
@FGYFFFF FGYFFFF force-pushed the fix/handler_middleware_name branch from 1c87f8d to 4db85f6 Compare March 13, 2023 02:43
@welkeyever
Copy link
Member

_a := root.Group("/a", _aMw()...)
如果是这样的命名风格,实际上如果有注册/a/a/c的场景,就变成:
_a := _a.Group("/a", _aMw()...)
_a.POST("/c", append(_method2Mw(), hertz.Method2)...)
了?

@FGYFFFF
Copy link
Contributor Author

FGYFFFF commented Mar 17, 2023

_a := root.Group("/a", _aMw()...) 如果是这样的命名风格,实际上如果有注册/a/a/c的场景,就变成: _a := _a.Group("/a", _aMw()...) _a.POST("/c", append(_method2Mw(), hertz.Method2)...) 了?

这里解了命名冲突,会生成如下:

root := r.Group("/", rootMw()...)
{
_a := root.Group("/a", _aMw()...)
{
	_a0 := _a.Group("/a", _a0Mw()...)
	_a0.GET("/b", append(_method1Mw(), hertz.Method1)...)
}
}

cmd/hz/util/data.go Outdated Show resolved Hide resolved
@FGYFFFF FGYFFFF force-pushed the fix/handler_middleware_name branch from 8c6acbf to e2a5647 Compare March 21, 2023 09:14
@FGYFFFF FGYFFFF changed the title fix(hz): handler middleware name feat(hz): distinction between "group middleware name" and "processor middleware name" Mar 21, 2023
@FGYFFFF FGYFFFF changed the title feat(hz): distinction between "group middleware name" and "processor middleware name" feat(hz): add middleware name style Mar 21, 2023
@FGYFFFF FGYFFFF force-pushed the fix/handler_middleware_name branch 3 times, most recently from 72fff5f to 917aca1 Compare March 27, 2023 11:52
@FGYFFFF FGYFFFF changed the title feat(hz): add middleware name style feat(hz): optimize the naming style for middleware. Mar 28, 2023
@FGYFFFF FGYFFFF changed the title feat(hz): optimize the naming style for middleware. feat(hz): optimize the naming style for middleware Mar 28, 2023
@FGYFFFF FGYFFFF force-pushed the fix/handler_middleware_name branch from 917aca1 to 389ada3 Compare March 28, 2023 07:12
@FGYFFFF FGYFFFF force-pushed the fix/handler_middleware_name branch from 389ada3 to f294e02 Compare March 28, 2023 07:29
@FGYFFFF FGYFFFF force-pushed the fix/handler_middleware_name branch from f294e02 to 174d08e Compare March 30, 2023 07:06
@FGYFFFF FGYFFFF force-pushed the fix/handler_middleware_name branch 2 times, most recently from f2d9047 to 5f6dc38 Compare March 31, 2023 06:30
@FGYFFFF FGYFFFF force-pushed the fix/handler_middleware_name branch from 5f6dc38 to 2c1f151 Compare March 31, 2023 07:30
@FGYFFFF FGYFFFF merged commit 1906ecb into cloudwego:develop Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants