AlexStocks commented on code in PR #3252:
URL: https://github.com/apache/dubbo-go/pull/3252#discussion_r3004222539
##########
cluster/router/chain/chain.go:
##########
@@ -110,6 +111,83 @@ func (c *RouterChain) copyRouters()
[]router.PriorityRouter {
return ret
}
+// getApplicationName gets the application name from URL with fallback to
SubURL
+func getApplicationName(url *common.URL) string {
+ appName := url.GetParam(constant.ApplicationKey, "")
+ if appName == "" && url.SubURL != nil {
+ appName = url.SubURL.GetParam(constant.ApplicationKey, "")
+ }
+ return appName
+}
+
+// isRouterMatch checks if router configuration matches the current
service/application
+func isRouterMatch(routerCfg *global.RouterConfig, url *common.URL, appName
string) bool {
+ switch routerCfg.Scope {
+ case constant.RouterScopeService:
+ if url.SubURL != nil {
+ return routerCfg.Key == url.SubURL.ServiceKey()
+ }
+ return routerCfg.Key == url.ServiceKey()
+ case constant.RouterScopeApplication:
+ return routerCfg.Key == appName
+ default:
+ logger.Warnf("unknown router scope: %s", routerCfg.Scope)
+ return false
+ }
+}
+
+// injectStaticRouters injects static router configurations into the router
chain.
+// Called after all routers are created to ensure they exist.
+// The injected static configs act as bootstrap state only during
initialization. For the shared
+// static and dynamic lifecycle semantics, see router.StaticConfigSetter.
+func (c *RouterChain) injectStaticRouters(url *common.URL) {
+ staticRoutersAttrAny, ok := url.GetAttribute(constant.RoutersConfigKey)
+ if !ok && url.SubURL != nil {
+ staticRoutersAttrAny, ok =
url.SubURL.GetAttribute(constant.RoutersConfigKey)
+ }
+ if !ok {
+ return
+ }
+ staticRoutersAttr, ok := staticRoutersAttrAny.([]*global.RouterConfig)
+ if !ok {
+ logger.Warnf("failed to type assert routers config: expected
[]*global.RouterConfig, got %T", staticRoutersAttrAny)
+ return
+ }
+ if len(staticRoutersAttr) == 0 {
+ return
+ }
+
+ applicationName := getApplicationName(url)
+
+ for _, routerCfg := range staticRoutersAttr {
+ if routerCfg == nil {
+ continue
+ }
+ if routerCfg.Enabled != nil && !*routerCfg.Enabled {
+ continue
+ }
+ if routerCfg.Valid != nil && !*routerCfg.Valid {
+ continue
+ }
+ if !isRouterMatch(routerCfg, url, applicationName) {
+ continue
+ }
+ c.injectRouterConfig(routerCfg)
+ }
+}
+
Review Comment:
[P0] 并发安全问题。injectStaticRouters 持有读锁时调用 injectRouterConfig，后者又获取读锁。虽然 RLock
可重入，但设计不清晰。建议 injectStaticRouters 不加锁，由 injectRouterConfig 统一加锁。
##########
cluster/router/chain/chain.go:
##########
@@ -110,6 +111,83 @@ func (c *RouterChain) copyRouters()
[]router.PriorityRouter {
return ret
}
+// getApplicationName gets the application name from URL with fallback to
SubURL
+func getApplicationName(url *common.URL) string {
+ appName := url.GetParam(constant.ApplicationKey, "")
+ if appName == "" && url.SubURL != nil {
+ appName = url.SubURL.GetParam(constant.ApplicationKey, "")
+ }
+ return appName
+}
+
+// isRouterMatch checks if router configuration matches the current
service/application
+func isRouterMatch(routerCfg *global.RouterConfig, url *common.URL, appName
string) bool {
+ switch routerCfg.Scope {
+ case constant.RouterScopeService:
+ if url.SubURL != nil {
+ return routerCfg.Key == url.SubURL.ServiceKey()
+ }
+ return routerCfg.Key == url.ServiceKey()
+ case constant.RouterScopeApplication:
+ return routerCfg.Key == appName
+ default:
+ logger.Warnf("unknown router scope: %s", routerCfg.Scope)
+ return false
+ }
+}
+
+// injectStaticRouters injects static router configurations into the router
chain.
+// Called after all routers are created to ensure they exist.
+// The injected static configs act as bootstrap state only during
initialization. For the shared
+// static and dynamic lifecycle semantics, see router.StaticConfigSetter.
+func (c *RouterChain) injectStaticRouters(url *common.URL) {
+ staticRoutersAttrAny, ok := url.GetAttribute(constant.RoutersConfigKey)
+ if !ok && url.SubURL != nil {
+ staticRoutersAttrAny, ok =
url.SubURL.GetAttribute(constant.RoutersConfigKey)
+ }
+ if !ok {
+ return
+ }
+ staticRoutersAttr, ok := staticRoutersAttrAny.([]*global.RouterConfig)
Review Comment:
[P1] 类型断言失败只记录警告，可能导致配置未生效而用户不知情。建议返回 error 或记录 error 级别日志。
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
