You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2020/07/29 07:34:09 UTC
[GitHub] [dubbo-go] zouyx commented on a change in pull request #662: [WIP]Ftr: Feature/application and service level router
zouyx commented on a change in pull request #662:
URL: https://github.com/apache/dubbo-go/pull/662#discussion_r461300280
##########
File path: cluster/router/condition/file.go
##########
@@ -71,11 +72,41 @@ func (f *FileConditionRouter) URL() common.URL {
common.WithParamsValue(constant.RouterPriority, strconv.Itoa(routerRule.Priority)),
common.WithParamsValue(constant.RULE_KEY, base64.URLEncoding.EncodeToString([]byte(rule))),
common.WithParamsValue(constant.ROUTER_KEY, constant.CONDITION_ROUTE_PROTOCOL),
- common.WithParamsValue(constant.CATEGORY_KEY, constant.ROUTERS_CATEGORY))
+ common.WithParamsValue(constant.CATEGORY_KEY, constant.ROUTERS_CATEGORY),
+ )
+ if routerRule.Scope == constant.RouterApplicationScope {
+ f.url.AddParam(constant.APPLICATION_KEY, routerRule.Key)
+ } else {
+ grp, srv, ver, _ := parseServiceRouterKey(routerRule.Key)
+ if len(grp) > 0 {
+ f.url.AddParam(constant.GROUP_KEY, grp)
+ }
+ if len(ver) > 0 {
+ f.url.AddParam(constant.VERSION_KEY, ver)
+ }
+ if len(srv) > 0 {
+ f.url.AddParam(constant.INTERFACE_KEY, srv)
+ }
+ }
})
return f.url
}
+func parseServiceRouterKey(key string) (string, string, string, error) {
+ if len(strings.TrimSpace(key)) == 0 {
+ return "", "", "", nil
+ }
+ reg := regexp.MustCompile(`(.*/{1})?([^:/]+)(:{1}[^:]*)?`)
+ strs := reg.FindAllStringSubmatch(key, -1)
+ if strs == nil || len(strs) != 1 {
Review comment:
```suggestion
if strs == nil || len(strs) > 1 {
```
you mean greater than 1 or greater than 0?
##########
File path: cluster/router/condition/file.go
##########
@@ -71,11 +72,41 @@ func (f *FileConditionRouter) URL() common.URL {
common.WithParamsValue(constant.RouterPriority, strconv.Itoa(routerRule.Priority)),
common.WithParamsValue(constant.RULE_KEY, base64.URLEncoding.EncodeToString([]byte(rule))),
common.WithParamsValue(constant.ROUTER_KEY, constant.CONDITION_ROUTE_PROTOCOL),
- common.WithParamsValue(constant.CATEGORY_KEY, constant.ROUTERS_CATEGORY))
+ common.WithParamsValue(constant.CATEGORY_KEY, constant.ROUTERS_CATEGORY),
+ )
+ if routerRule.Scope == constant.RouterApplicationScope {
+ f.url.AddParam(constant.APPLICATION_KEY, routerRule.Key)
+ } else {
+ grp, srv, ver, _ := parseServiceRouterKey(routerRule.Key)
+ if len(grp) > 0 {
+ f.url.AddParam(constant.GROUP_KEY, grp)
+ }
+ if len(ver) > 0 {
+ f.url.AddParam(constant.VERSION_KEY, ver)
+ }
+ if len(srv) > 0 {
+ f.url.AddParam(constant.INTERFACE_KEY, srv)
+ }
+ }
})
return f.url
}
+func parseServiceRouterKey(key string) (string, string, string, error) {
+ if len(strings.TrimSpace(key)) == 0 {
+ return "", "", "", nil
+ }
+ reg := regexp.MustCompile(`(.*/{1})?([^:/]+)(:{1}[^:]*)?`)
+ strs := reg.FindAllStringSubmatch(key, -1)
+ if strs == nil || len(strs) != 1 {
+ return "", "", "", perrors.Errorf("Invalid key, service key must follow [{group}/]{service}[:{version}] pattern")
+ }
+ grp := strings.TrimSpace(strings.TrimRight(strs[0][1], "/"))
+ srv := strings.TrimSpace(strs[0][2])
+ ver := strings.TrimSpace(strings.TrimLeft(strs[0][3], ":"))
Review comment:
Should you check `len(strs[0]) ==3 ` or `len(strs[0]) >2 `
##########
File path: cluster/router/condition/router_rule.go
##########
@@ -57,7 +58,7 @@ func getRule(rawRule string) (*RouterRule, error) {
return r, err
}
r.RawRule = rawRule
- if len(r.Conditions) != 0 {
+ if len(r.Conditions) != 0 && r.Key != "" && (r.Scope == constant.RouterApplicationScope || r.Scope == constant.RouterServiceScope) {
Review comment:
```suggestion
if len(r.Conditions) > 0 && len(r.Key)>0 && (r.Scope == constant.RouterApplicationScope || r.Scope == constant.RouterServiceScope) {
```
##########
File path: cluster/router/condition/router.go
##########
@@ -117,7 +117,13 @@ func NewConditionRouter(url *common.URL) (*ConditionRouter, error) {
}
router.url = url
- router.priority = url.GetParamInt(constant.RouterPriority, 0)
+ var defaultPriority int64
+ if url.GetParam(constant.APPLICATION_KEY, "") != "" {
+ defaultPriority = 150
+ } else {
+ defaultPriority = 140
Review comment:
why change default priotiry ?
##########
File path: config/router_config.go
##########
@@ -32,16 +33,37 @@ var (
routerURLSet = gxset.NewSet()
)
+// LocalRouterRules defines the local router config structure
+type LocalRouterRules struct {
+ RouterRules []interface{} `yaml:"routerRules"`
+}
+
// RouterInit Load config file to init router config
func RouterInit(confRouterFile string) error {
- fileRouterFactories := extension.GetFileRouterFactories()
bytes, err := yaml.LoadYMLConfig(confRouterFile)
if err != nil {
return perrors.Errorf("ioutil.ReadFile(file:%s) = error:%v", confRouterFile, perrors.WithStack(err))
}
- logger.Warnf("get fileRouterFactories len{%+v})", len(fileRouterFactories))
- for k, factory := range fileRouterFactories {
- r, e := factory.NewFileRouter(bytes)
+ routerRules := &LocalRouterRules{}
+ err = yaml.UnmarshalYML(bytes, routerRules)
+ if err != nil {
+ return perrors.Errorf("Load router file %s failed due to error: %v", confRouterFile, perrors.WithStack(err))
+ }
+ if len(routerRules.RouterRules) == 0 {
+ return perrors.Errorf("No router configurations in file %s", confRouterFile)
+ }
+ fileRouterFactories := extension.GetFileRouterFactories()
+ for _, v := range routerRules.RouterRules {
+ content, _ := yaml.MarshalYML(v)
+ err = initRouterConfig(content, fileRouterFactories)
+ }
+ return err
Review comment:
i think should add a interface here, for load the router config file.
##########
File path: cluster/router/condition/file.go
##########
@@ -71,11 +72,41 @@ func (f *FileConditionRouter) URL() common.URL {
common.WithParamsValue(constant.RouterPriority, strconv.Itoa(routerRule.Priority)),
common.WithParamsValue(constant.RULE_KEY, base64.URLEncoding.EncodeToString([]byte(rule))),
common.WithParamsValue(constant.ROUTER_KEY, constant.CONDITION_ROUTE_PROTOCOL),
- common.WithParamsValue(constant.CATEGORY_KEY, constant.ROUTERS_CATEGORY))
+ common.WithParamsValue(constant.CATEGORY_KEY, constant.ROUTERS_CATEGORY),
+ )
+ if routerRule.Scope == constant.RouterApplicationScope {
+ f.url.AddParam(constant.APPLICATION_KEY, routerRule.Key)
+ } else {
+ grp, srv, ver, _ := parseServiceRouterKey(routerRule.Key)
+ if len(grp) > 0 {
+ f.url.AddParam(constant.GROUP_KEY, grp)
+ }
+ if len(ver) > 0 {
+ f.url.AddParam(constant.VERSION_KEY, ver)
+ }
+ if len(srv) > 0 {
+ f.url.AddParam(constant.INTERFACE_KEY, srv)
+ }
+ }
Review comment:
```suggestion
if routerRule.Scope == constant.RouterApplicationScope {
f.url.AddParam(constant.APPLICATION_KEY, routerRule.Key)
return
}
grp, srv, ver, e := parseServiceRouterKey(routerRule.Key)
if e != nil {
return
}
if len(grp) > 0 {
f.url.AddParam(constant.GROUP_KEY, grp)
}
if len(ver) > 0 {
f.url.AddParam(constant.VERSION_KEY, ver)
}
if len(srv) > 0 {
f.url.AddParam(constant.INTERFACE_KEY, srv)
}
```
##########
File path: cluster/router/condition/file.go
##########
@@ -71,11 +72,41 @@ func (f *FileConditionRouter) URL() common.URL {
common.WithParamsValue(constant.RouterPriority, strconv.Itoa(routerRule.Priority)),
common.WithParamsValue(constant.RULE_KEY, base64.URLEncoding.EncodeToString([]byte(rule))),
common.WithParamsValue(constant.ROUTER_KEY, constant.CONDITION_ROUTE_PROTOCOL),
- common.WithParamsValue(constant.CATEGORY_KEY, constant.ROUTERS_CATEGORY))
+ common.WithParamsValue(constant.CATEGORY_KEY, constant.ROUTERS_CATEGORY),
+ )
+ if routerRule.Scope == constant.RouterApplicationScope {
+ f.url.AddParam(constant.APPLICATION_KEY, routerRule.Key)
+ } else {
+ grp, srv, ver, _ := parseServiceRouterKey(routerRule.Key)
+ if len(grp) > 0 {
+ f.url.AddParam(constant.GROUP_KEY, grp)
+ }
+ if len(ver) > 0 {
+ f.url.AddParam(constant.VERSION_KEY, ver)
+ }
+ if len(srv) > 0 {
+ f.url.AddParam(constant.INTERFACE_KEY, srv)
+ }
+ }
})
return f.url
}
+func parseServiceRouterKey(key string) (string, string, string, error) {
+ if len(strings.TrimSpace(key)) == 0 {
+ return "", "", "", nil
+ }
+ reg := regexp.MustCompile(`(.*/{1})?([^:/]+)(:{1}[^:]*)?`)
+ strs := reg.FindAllStringSubmatch(key, -1)
+ if strs == nil || len(strs) != 1 {
+ return "", "", "", perrors.Errorf("Invalid key, service key must follow [{group}/]{service}[:{version}] pattern")
+ }
+ grp := strings.TrimSpace(strings.TrimRight(strs[0][1], "/"))
+ srv := strings.TrimSpace(strs[0][2])
+ ver := strings.TrimSpace(strings.TrimLeft(strs[0][3], ":"))
Review comment:
Should you check `len(strs[0]) ==4 ` or `len(strs[0]) >3 `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org