Skip to content

Commit

Permalink
optimize(hz): sort route strictly which preventing sorting inconsiste…
Browse files Browse the repository at this point in the history
…ncies (#921)

#### What type of PR is this?
<!--
Add one of the following kinds:

build: Changes that affect the build system or external dependencies
(example scopes: gulp, broccoli, npm)
ci: Changes to our CI configuration files and scripts (example scopes:
Travis, Circle, BrowserStack, SauceLabs)
docs: Documentation only changes
feat: A new feature
optimize: A new optimization
fix: A bug fix
perf: A code change that improves performance
refactor: A code change that neither fixes a bug nor adds a feature
style: Changes that do not affect the meaning of the code (white space,
formatting, missing semi-colons, etc)
test: Adding missing tests or correcting existing tests
chore: Changes to the build process or auxiliary tools and libraries
such as documentation generation
-->
optimize
#### Check the PR title.
<!--
The description of the title will be attached in Release Notes, 
so please describe it from user-oriented, what this PR does / why we
need it.
Please check your PR title with the below requirements:
-->
- [x] This PR title match the format: \<type\>(optional scope):
\<description\>
- [x] The description of this PR title is user-oriented and clear enough
for others to understand.
- [ ] Attach the PR updating the user documentation if the current PR
requires user awareness at the usage level. [User docs
repo](https://github.com/cloudwego/cloudwego.github.io)


#### (Optional) Translate the PR title into Chinese.
对路由进行严格排序,防止生成代码出现大量的 diff

#### (Optional) More detailed description for this PR(en: English/zh:
Chinese).
<!--
Provide more detailed info for review(e.g., it's recommended to provide
perf data if this is a perf type PR).
-->
en: Strict ordering of routes to prevent generating code with lots of
diffs
zh(optional): 对路由进行严格排序,防止生成的路由代码出现由于顺序带来的 diff
之前的路由在排序的时候漏掉三个case:
- "/:b/a": 这种情况会取 ":b" 与其他节点进行比较,导致路由生成不是按照路由首字母来排序
- "get /a", "post /a": 这种情况之前默认使用 "a" 和 "a" 进行比较,导致生成的路由 get/post
等方法可能出现不一致的顺序,从而生成代码出现 diff
- 之前构建路由树的时候,结构和用户定义接口的顺序有关,导致用户添加一个接口时,生成代码顺序发生变化,针对这个情况也做了排序
- 之前漏掉对于中间节点的排序(路由组节点的排序),导致可能会出现生成代码的diff

本 pr 的修改不会影响路由注册的逻辑,但是会使得之前生成的路由代码的顺序发生改变(在idl的method特别多的时候表现明显)。
在这个问题修好之后,以后所以的 idl 都不会对 router 带来diff(除了加路由)

#### (Optional) Which issue(s) this PR fixes:
<!--
Automatically closes linked issue when PR is merged.
Eg: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->

#### (Optional) The PR that updates user documentation:
<!--
If the current PR requires user awareness at the usage level, please
submit a PR to update user docs. [User docs
repo](https://github.com/cloudwego/cloudwego.github.io)
-->
  • Loading branch information
FGYFFFF committed May 9, 2024
2 parents bd2a9b2 + 9a95dc8 commit 6f62cd9
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 15 deletions.
3 changes: 3 additions & 0 deletions cmd/hz/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ func Init() *cli.App {
noRecurseFlag := cli.BoolFlag{Name: "no_recurse", Usage: "Generate master model only.", Destination: &globalArgs.NoRecurse}
forceNewFlag := cli.BoolFlag{Name: "force", Aliases: []string{"f"}, Usage: "Force new a project, which will overwrite the generated files", Destination: &globalArgs.ForceNew}
enableExtendsFlag := cli.BoolFlag{Name: "enable_extends", Usage: "Parse 'extends' for thrift IDL", Destination: &globalArgs.EnableExtends}
sortRouterFlag := cli.BoolFlag{Name: "sort_router", Usage: "Sort router register code, to avoid code difference", Destination: &globalArgs.SortRouter}

jsonEnumStrFlag := cli.BoolFlag{Name: "json_enumstr", Usage: "Use string instead of num for json enums when idl is thrift.", Destination: &globalArgs.JSONEnumStr}
queryEnumIntFlag := cli.BoolFlag{Name: "query_enumint", Usage: "Use num instead of string for query enum parameter.", Destination: &globalArgs.QueryEnumAsInt}
Expand Down Expand Up @@ -227,6 +228,7 @@ func Init() *cli.App {
&noRecurseFlag,
&forceNewFlag,
&enableExtendsFlag,
&sortRouterFlag,

&jsonEnumStrFlag,
&unsetOmitemptyFlag,
Expand Down Expand Up @@ -261,6 +263,7 @@ func Init() *cli.App {
&optPkgFlag,
&noRecurseFlag,
&enableExtendsFlag,
&sortRouterFlag,

&jsonEnumStrFlag,
&unsetOmitemptyFlag,
Expand Down
1 change: 1 addition & 0 deletions cmd/hz/config/argument.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ type Argument struct {
ForceNew bool
SnakeStyleMiddleware bool
EnableExtends bool
SortRouter bool

CustomizeLayout string
CustomizeLayoutData string
Expand Down
2 changes: 1 addition & 1 deletion cmd/hz/generator/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func (pkgGen *HttpPackageGenerator) processHandler(handler *Handler, root *Route
}
handler.Imports[mm.PackageName] = mm
}
err := root.Update(m, handler.PackageName, singleHandlerPackage)
err := root.Update(m, handler.PackageName, singleHandlerPackage, pkgGen.SortRouter)
if err != nil {
return err
}
Expand Down
3 changes: 2 additions & 1 deletion cmd/hz/generator/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,13 @@ type HttpPackageGenerator struct {
IdlClientDir string // client dir for "client" command
ForceClientDir string // client dir without namespace for "client" command
BaseDomain string // request domain for "client" command
QueryEnumAsInt bool // client code use number for query parameter
QueryEnumAsInt bool // client code use number for query parameter
ServiceGenDir string

NeedModel bool
HandlerByMethod bool // generate handler files with method dimension
SnakeStyleMiddleware bool // use snake name style for middleware
SortRouter bool

loadedBackend Backend
curModel *model.Model
Expand Down
101 changes: 90 additions & 11 deletions cmd/hz/generator/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@ import (
"bytes"
"fmt"
"io/ioutil"
"math"
"path/filepath"
"regexp"
"sort"
"strconv"
"strings"
"unicode"

"github.com/cloudwego/hertz/cmd/hz/util"
)
Expand Down Expand Up @@ -73,20 +76,20 @@ func (routerNode *RouterNode) Sort() {
sort.Sort(routerNode.Children)
}

func (routerNode *RouterNode) Update(method *HttpMethod, handlerType, handlerPkg string) error {
func (routerNode *RouterNode) Update(method *HttpMethod, handlerType, handlerPkg string, sortRouter bool) error {
if method.Path == "" {
return fmt.Errorf("empty path for method '%s'", method.Name)
}
paths := strings.Split(method.Path, "/")
if paths[0] == "" {
paths = paths[1:]
}
parent, last := routerNode.FindNearest(paths)
parent, last := routerNode.FindNearest(paths, method.HTTPMethod, sortRouter)
if last == len(paths) {
return fmt.Errorf("path '%s' has been registered", method.Path)
}
name := util.ToVarName(paths[:last])
parent.Insert(name, method, handlerType, paths[last:], handlerPkg)
parent.Insert(name, method, handlerType, paths[last:], handlerPkg, sortRouter)
parent.Sort()
return nil
}
Expand Down Expand Up @@ -192,7 +195,7 @@ func (routerNode *RouterNode) DFS(i int, hook func(layer int, node *RouterNode)

var handlerPkgMap map[string]string

func (routerNode *RouterNode) Insert(name string, method *HttpMethod, handlerType string, paths []string, handlerPkg string) {
func (routerNode *RouterNode) Insert(name string, method *HttpMethod, handlerType string, paths []string, handlerPkg string, sortRouter bool) {
cur := routerNode
for i, p := range paths {
c := &RouterNode{
Expand Down Expand Up @@ -229,6 +232,9 @@ func (routerNode *RouterNode) Insert(name string, method *HttpMethod, handlerTyp
cur.Children = make([]*RouterNode, 0, 1)
}
cur.Children = append(cur.Children, c)
if sortRouter {
sort.Sort(cur.Children)
}
cur = c
}
}
Expand All @@ -240,14 +246,21 @@ func getHttpMethod(method string) string {
return strings.ToUpper(method)
}

func (routerNode *RouterNode) FindNearest(paths []string) (*RouterNode, int) {
func (routerNode *RouterNode) FindNearest(paths []string, method string, sortRouter bool) (*RouterNode, int) {
ns := len(paths)
cur := routerNode
i := 0
path := paths[i]
for j := 0; j < len(cur.Children); j++ {
c := cur.Children[j]
tmpMethod := "" // group do not have http method
if i == ns { // only i==ns, the path is http method node
tmpMethod = method
}
if ("/" + path) == c.Path {
if sortRouter && strings.EqualFold(c.HttpMethod, tmpMethod) {
continue
}
i++
if i == ns {
return cur, i - 1
Expand All @@ -270,17 +283,35 @@ func (c childrenRouterInfo) Len() int {
// Less reports whether the element with
// index i should sort before the element with index j.
func (c childrenRouterInfo) Less(i, j int) bool {
ci := c[i].Path
if len(c[i].Children) != 0 {
ci = ci[1:]
if c[i].HttpMethod == "" && c[j].HttpMethod != "" {
return false
}
if c[i].HttpMethod != "" && c[j].HttpMethod == "" {
return true
}
cj := c[j].Path
if len(c[j].Children) != 0 {
cj = cj[1:]
// remove non-litter char
// eg. /a -> a
// /:a -> a
ci := removeNonLetterPrefix(c[i].Path)
cj := removeNonLetterPrefix(c[j].Path)

// if ci == cj, use HTTP mothod for sort, preventing sorting inconsistencies
if ci == cj {
return c[i].HttpMethod < c[j].HttpMethod
}

return ci < cj
}

func removeNonLetterPrefix(str string) string {
for i, char := range str {
if unicode.IsLetter(char) || unicode.IsDigit(char) {
return str[i:]
}
}
return str
}

// Swap swaps the elements with indexes i and j.
func (c childrenRouterInfo) Swap(i, j int) {
c[i], c[j] = c[j], c[i]
Expand Down Expand Up @@ -341,6 +372,29 @@ func (pkgGen *HttpPackageGenerator) updateRegister(pkg, rDir, pkgName string) er
return nil
}

func appendMw(mws []string, mw string) ([]string, string) {
for i := 0; true; i++ {
if i == math.MaxInt {
break
}
if !stringsIncludes(mws, mw) {
mws = append(mws, mw)
break
}
mw += strconv.Itoa(i)
}
return mws, mw
}

func stringsIncludes(strs []string, str string) bool {
for _, s := range strs {
if s == str {
return true
}
}
return false
}

func (pkgGen *HttpPackageGenerator) genRouter(pkg *HttpPackage, root *RouterNode, handlerPackage, routerDir, routerPackage string) error {
err := root.DyeGroupName(pkgGen.SnakeStyleMiddleware)
if err != nil {
Expand All @@ -367,6 +421,31 @@ func (pkgGen *HttpPackageGenerator) genRouter(pkg *HttpPackage, root *RouterNode
router.HandlerPackages = handlerMap
}

if pkgGen.SnakeStyleMiddleware { // unique middleware name for SnakeStyleMiddleware
mws := []string{}
hook := func(layer int, node *RouterNode) error {
if len(node.Children) == 0 {
return nil
}
groupMwName := node.GroupMiddleware
handlerMwName := node.HandlerMiddleware
if len(groupMwName) != 0 {
mws, groupMwName = appendMw(mws, groupMwName)
}
if len(handlerMwName) != 0 {
mws, handlerMwName = appendMw(mws, handlerMwName)
}
if groupMwName != node.GroupMiddleware {
node.GroupMiddleware = groupMwName
}
if handlerMwName != node.HandlerMiddleware {
node.HandlerMiddleware = handlerMwName
}
return nil
}
root.DFS(0, hook)
}

// store router info
pkg.RouterInfo = &router

Expand Down
1 change: 1 addition & 0 deletions cmd/hz/protobuf/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,7 @@ func (plugin *Plugin) genHttpPackage(ast *descriptorpb.FileDescriptorProto, deps
BaseDomain: args.BaseDomain,
QueryEnumAsInt: args.QueryEnumAsInt,
SnakeStyleMiddleware: args.SnakeStyleMiddleware,
SortRouter: args.SortRouter,
}

if args.ModelBackend != "" {
Expand Down
1 change: 1 addition & 0 deletions cmd/hz/thrift/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ func (plugin *Plugin) Run() int {
BaseDomain: args.BaseDomain,
QueryEnumAsInt: args.QueryEnumAsInt,
SnakeStyleMiddleware: args.SnakeStyleMiddleware,
SortRouter: args.SortRouter,
}
if args.ModelBackend != "" {
sg.Backend = meta.Backend(args.ModelBackend)
Expand Down
13 changes: 11 additions & 2 deletions cmd/hz/thrift/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,21 @@ func TestRun(t *testing.T) {
HandlerDir: handlerDir,
RouterDir: routerDir,
ModelDir: modelDir,
UseDir: args.Use,
ClientDir: clientDir,
TemplateGenerator: generator.TemplateGenerator{
OutputDir: args.OutDir,
Excludes: args.Excludes,
},
ProjPackage: pkg,
Options: options,
ProjPackage: pkg,
Options: options,
HandlerByMethod: args.HandlerByMethod,
CmdType: args.CmdType,
ForceClientDir: args.ForceClientDir,
BaseDomain: args.BaseDomain,
QueryEnumAsInt: args.QueryEnumAsInt,
SnakeStyleMiddleware: args.SnakeStyleMiddleware,
SortRouter: args.SortRouter,
}
if args.ModelBackend != "" {
sg.Backend = meta.Backend(args.ModelBackend)
Expand Down

0 comments on commit 6f62cd9

Please sign in to comment.