AlexStocks commented on code in PR #3252:
URL: https://github.com/apache/dubbo-go/pull/3252#discussion_r2943063921
##########
cluster/router/chain/chain.go:
##########
@@ -106,6 +107,81 @@ 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.
+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)
+ }
+}
+
+// injectRouterConfig injects router configuration into routers that implement
StaticConfigSetter.
+// Each router decides whether the config applies to it and no-ops if not.
+func (c *RouterChain) injectRouterConfig(routerCfg *global.RouterConfig) {
+ c.mutex.RLock()
+ defer c.mutex.RUnlock()
+ for _, r := range c.routers {
+ if setter, ok := r.(router.StaticConfigSetter); ok {
+ setter.SetStaticConfig(routerCfg)
+ }
+ }
+}
+
// NewRouterChain init router chain
Review Comment:
injectRouterConfig 里直接把外部传入的 routerCfg 指针传给
setter.SetStaticConfig(routerCfg)，没有做拷贝。虽然 tag router 的 SetStaticConfig 内部调了
cfg.Clone()，但 condition router 的 SetStaticConfig 没有 clone，直接读 cfg.Conditions 和
cfg.Force。
如果将来有人在外部修改了这个 RouterConfig（比如动态更新场景），condition router 会受影响。要么在
injectRouterConfig 统一 clone 一次再传给 setter，要么在 StaticConfigSetter 接口文档里强制要求实现方必须
clone，并且 condition router 也要补上。
##########
cluster/router/condition/dynamic_router.go:
##########
@@ -134,6 +134,44 @@ func (d *DynamicRouter) URL() *common.URL {
return nil
}
+// SetStaticConfig applies a RouterConfig with string conditions directly,
bypassing YAML parsing.
+// This is the correct entry point for static (code-configured) rules;
+// Process is designed for dynamic config-center updates that arrive as YAML
text.
+func (d *DynamicRouter) SetStaticConfig(cfg *global.RouterConfig) {
+ if cfg == nil || len(cfg.Conditions) == 0 {
+ return
+ }
+
+ d.mu.Lock()
+ defer d.mu.Unlock()
+
+ force := cfg.Force != nil && *cfg.Force
+ enable := cfg.Enabled == nil || *cfg.Enabled
+
+ if !enable {
+ d.force, d.enable, d.conditionRouter = force, false, nil
+ return
+ }
+
+ conditionRouters := make([]*StateRouter, 0, len(cfg.Conditions))
+ for _, conditionRule := range cfg.Conditions {
+ url, err := common.NewURL("condition://")
+ if err != nil {
+ logger.Warnf("[condition router] failed to create
condition URL: %v", err)
+ continue
+ }
+ url.AddParam(constant.RuleKey, conditionRule)
+ url.AddParam(constant.ForceKey, strconv.FormatBool(force))
+ conditionRoute, err := NewConditionStateRouter(url)
+ if err != nil {
+ logger.Warnf("[condition router] failed to parse
condition rule %q: %v", conditionRule, err)
+ continue
+ }
+ conditionRouters = append(conditionRouters, conditionRoute)
+ }
+ d.force, d.enable, d.conditionRouter = force, enable,
stateRouters(conditionRouters)
+}
+
func (d *DynamicRouter) Process(event *config_center.ConfigChangeEvent) {
d.mu.Lock()
defer d.mu.Unlock()
Review Comment:
静态注入和动态配置中心的关系没说清楚。DynamicRouter.Process() 处理配置中心的动态更新，SetStaticConfig()
处理静态注入，两者都会修改 d.conditionRouter。如果同时存在，后执行的会覆盖前面的。
建议：
1. 在注释里明确说明优先级：是动态覆盖静态，还是静态不可被动态覆盖？
2. 或者考虑分开存储静态和动态规则，Route 时合并执行
##########
client/options.go:
##########
@@ -546,6 +555,17 @@ func setRegistries(regs map[string]*global.RegistryConfig)
ReferenceOption {
}
}
+func setRouters(routers []*global.RouterConfig) ReferenceOption {
+ return func(opts *ReferenceOptions) {
+ if len(routers) > 0 {
+ if opts.Routers == nil {
+ opts.Routers = make([]*global.RouterConfig, 0)
+ }
+ opts.Routers = append(opts.Routers, routers...)
+ }
+ }
+}
Review Comment:
Alanxtl 之前提的问题还没回应：setRouters 是 append、WithClientRouter 是
append、SetClientRouters 是直接覆盖。三种语义混在一起，调用方很容易踩坑。
建议统一语义：要么全部 append（SetClientRouters 也改成 append），要么在文档/注释里明确标注哪个是 append 哪个是
replace，以及框架内部调用顺序是什么。
--
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]
