AlexStocks commented on code in PR #3687:
URL: https://github.com/apache/dubbo-go/pull/3687#discussion_r3985798181
##########
server/options.go:
##########
@@ -96,6 +99,12 @@ func (srvOpts *ServerOptions) init(opts ...ServerOption)
error {
return err
}
+ filterNames, err := extension.Initialize(srvOpts.extensionConfigs,
srvOpts.extensionOptions, extension.ServerScope)
+ if err != nil {
+ return err
+ }
+ providerConf.Filter = extension.MergeFilterNames(providerConf.Filter,
filterNames)
Review Comment:
[P0] 服务端把扩展 filter 合并进 Provider.Filter，会整体替换默认 provider filter 链
`providerConf.Filter` 默认是空串；`server/action.go:373` 的判定是
`if svcConf.Filter == "" { filters = constant.DefaultServiceFilters } else {
filters = svcConf.Filter }`，
而 `ServiceOptions.init` 通过 `dubboutil.CopyFields` 在 `Service.Filter` 为空时把
`Provider.Filter` 抄进去。
所以这里一旦写入扩展贡献的 filter 名，导出 URL 的 `service.filter`
就变成非空，`DefaultServiceFilters` 被整体丢弃。
在 exact Head 1bc7adaf 上按 `defaultServerOptions().init` ->
`defaultServiceOptions().init` -> `getUrlMap()`（与 `Server.genSvcOpts` /
`Export` 同一条链）做探针：
- 不启用扩展：`service.filter =
"echo,token,accesslog,tps,generic_service,execute,pshutdown"`
- 启用一个只贡献 1 个 filter 名的扩展：`service.filter =
"probe-provider-filter"`，`echo`/`token`/`pshutdown` 全部消失
消费端没有这个问题：`client/action.go:444` 用 `MergeValue(ref.Filter, "",
defaultReferenceFilter)` 把扩展 filter 并入默认值，实测为
`cshutdown,probe-consumer-filter`，两端语义不一致。
被丢掉的链里包含 `token`（provider 侧 token 校验，`filter/token/filter.go` 在 `TokenKey`
非空时强制校验）和 `tps`/`execute`（限流与并发限制），因此只要业务启用任意声明 filter 的扩展，服务端默认鉴权与限流就会静默失效。
建议让 `Provider.Filter` 只承载用户显式配置，扩展名在确定 `DefaultServiceFilters` 之后再合并（或复用
`commonCfg.MergeValue` 的语义），并补一条断言导出 URL `service.filter` 的用例。
##########
server/options_test.go:
##########
@@ -29,11 +30,141 @@ import (
import (
"dubbo.apache.org/dubbo-go/v3/common/constant"
+ "dubbo.apache.org/dubbo-go/v3/common/extension"
+ "dubbo.apache.org/dubbo-go/v3/filter"
"dubbo.apache.org/dubbo-go/v3/global"
"dubbo.apache.org/dubbo-go/v3/protocol"
"dubbo.apache.org/dubbo-go/v3/registry"
)
+type serverEntryConfig struct {
+ prefix string
+ Value int `yaml:"value"`
+ requiredScope extension.Scope
+ initialized extension.Scope
+ onInit func(*serverEntryConfig)
+}
+
+func (c *serverEntryConfig) Prefix() string {
+ return c.prefix
+}
+
+func (c *serverEntryConfig) New() extension.Config {
+ return &serverEntryConfig{
+ prefix: c.prefix,
+ Value: 1,
+ requiredScope: c.requiredScope,
+ onInit: c.onInit,
+ }
+}
+
+func (c *serverEntryConfig) Init(scope extension.Scope) error {
+ if c.requiredScope != 0 && scope != c.requiredScope {
+ return errors.New("server scope is required")
+ }
+ c.initialized = scope
+ if c.onInit != nil {
+ c.onInit(c)
+ }
+ return nil
+}
+
+func (c *serverEntryConfig) FilterNames(extension.Scope) []string {
+ return []string{"server-entry-filter"}
+}
+
+type serverEntryOption struct {
+ prefix string
+ value int
+}
+
+func (o serverEntryOption) Prefix() string {
+ return o.prefix
+}
+
+func (o serverEntryOption) Apply(config extension.Config) error {
+ config.(*serverEntryConfig).Value = o.value
+ return nil
+}
+
+func TestWithExtensionBuildsServerConfigAndMergesFilter(t *testing.T) {
+ const prefix = "server-entry"
+ const filterName = "server-entry-filter"
+
+ extension.UnregisterConfig(prefix)
+ extension.UnregisterFilter(filterName)
+ t.Cleanup(func() {
+ extension.UnregisterConfig(prefix)
+ extension.UnregisterFilter(filterName)
+ })
+
+ var initialized *serverEntryConfig
+ require.NoError(t, extension.RegisterConfig(&serverEntryConfig{
+ prefix: prefix,
+ requiredScope: extension.ServerScope,
+ onInit: func(config *serverEntryConfig) {
+ initialized = config
+ },
+ }))
+ extension.SetFilter(filterName, func() filter.Filter { return nil })
+
+ srv, err := NewServer(
+ SetServerExtensionConfigs(map[string]any{
+ prefix: map[string]any{
+ "provider": map[string]any{"value": 7},
+ },
+ }),
+ WithServerFilter("explicit"),
+ WithExtension(serverEntryOption{prefix: prefix, value: 9}),
+ )
+ require.NoError(t, err)
+ require.NotNil(t, srv)
+ require.NotNil(t, initialized)
+ assert.Equal(t, 9, initialized.Value)
+ assert.Equal(t, extension.ServerScope, initialized.initialized)
+ assert.Equal(t, "explicit,"+filterName, srv.cfg.Provider.Filter)
Review Comment:
[P2] 断言内部字段，没有覆盖真实的 filter 消费路径
这里断言的是 `srv.cfg.Provider.Filter`（包内字段），不是导出 URL 的 `service.filter`。扩展 filter
真正生效的位置是 `server/action.go:373` 的 `svcConf.Filter` 判定和
`protocolwrapper.BuildInvokerChain`，这条链没有任何用例经过，所以「扩展 filter 顶掉
DefaultServiceFilters」的回归不会被现有测试发现（本用例在 Provider.Filter
上是通过的）。`client/options_test.go:145` 断言的 `cli.cliOpts.overallReference.Filter`
同理，未经过 `MergeValue` 与 `BuildInvokerChain`。
建议从公开入口构造 server/client，断言导出 URL 上的 `service.filter` / `reference.filter`
集合，并覆盖「扩展贡献 filter + 用户未显式配置 provider filter」这一默认场景。
##########
dubbo_test.go:
##########
@@ -38,6 +39,131 @@ import (
"dubbo.apache.org/dubbo-go/v3/server"
)
+type instanceEntryConfig struct {
+ prefix string
+ Value int
+ initialized extension.Scope
+ onInit func(*instanceEntryConfig)
+}
+
+func (c *instanceEntryConfig) Prefix() string {
+ return c.prefix
+}
+
+func (c *instanceEntryConfig) New() extension.Config {
+ return &instanceEntryConfig{
+ prefix: c.prefix,
+ Value: 1,
+ onInit: c.onInit,
+ }
+}
+
+func (c *instanceEntryConfig) Init(scope extension.Scope) error {
+ if scope != extension.InstanceScope && scope != extension.ClientScope {
+ return errors.New("instance or client scope is required")
+ }
+ c.initialized = scope
+ if c.onInit != nil {
+ c.onInit(c)
+ }
+ return nil
+}
+
+func (c *instanceEntryConfig) FilterNames(extension.Scope) []string {
+ return nil
+}
+
+type instanceEntryOption struct {
+ prefix string
+ value int
+}
+
+func (o instanceEntryOption) Prefix() string {
+ return o.prefix
+}
+
+func (o instanceEntryOption) Apply(config extension.Config) error {
+ config.(*instanceEntryConfig).Value = o.value
+ return nil
+}
+
+type configCenterEntryConfig struct {
+ prefix string
+ LocalValue int `yaml:"local-value"`
+ RemoteValue int `yaml:"remote-value"`
+ onInit func(*configCenterEntryConfig)
+}
+
+func (c *configCenterEntryConfig) Prefix() string {
+ return c.prefix
+}
+
+func (c *configCenterEntryConfig) New() extension.Config {
+ return &configCenterEntryConfig{prefix: c.prefix, onInit: c.onInit}
+}
+
+func (c *configCenterEntryConfig) Init(scope extension.Scope) error {
+ if scope != extension.ClientScope {
+ return errors.New("client scope is required")
+ }
+ if c.onInit != nil {
+ c.onInit(c)
+ }
+ return nil
+}
+
+func (c *configCenterEntryConfig) FilterNames(extension.Scope) []string {
+ return nil
+}
+
+func TestWithExtensionBuildsInstanceConfig(t *testing.T) {
+ const prefix = "instance-entry"
+ extension.UnregisterConfig(prefix)
+ t.Cleanup(func() { extension.UnregisterConfig(prefix) })
+
+ var initialized *instanceEntryConfig
+ require.NoError(t, extension.RegisterConfig(&instanceEntryConfig{
+ prefix: prefix,
+ onInit: func(config *instanceEntryConfig) {
+ initialized = config
+ },
+ }))
+
+ _, err := NewInstance(WithExtension(instanceEntryOption{prefix: prefix,
value: 9}))
+ require.NoError(t, err)
+ require.NotNil(t, initialized)
+ assert.Equal(t, 9, initialized.Value)
+ assert.Equal(t, extension.InstanceScope, initialized.initialized)
+}
+
+func TestInstancePropagatesRoleSpecificExtensionYAMLToClient(t *testing.T) {
+ const prefix = "instance-to-client-entry"
+ extension.UnregisterConfig(prefix)
+ t.Cleanup(func() { extension.UnregisterConfig(prefix) })
+
+ var initialized *instanceEntryConfig
+ require.NoError(t, extension.RegisterConfig(&instanceEntryConfig{
+ prefix: prefix,
+ onInit: func(config *instanceEntryConfig) {
+ initialized = config
+ },
+ }))
+
+ instance, err := NewInstance(func(opts *InstanceOptions) {
+ opts.extensionConfigs = map[string]any{
Review Comment:
[P2] YAML 到运行时的端到端链路没有测试覆盖
本用例直接赋值 `opts.extensionConfigs`，绕过了 `loader.go` 的
`extensionConfigsFromKoanf`；`loader_test.go` 的
`TestExtensionConfigsFromKoanfPreservesDottedKeys` 只验证抽取函数本身。两端各自有测试，但 `Load()`
/ `hotUpdateConfig()` 里「先 `newOpts.extensionConfigs =
extensionConfigsFromKoanf(koan)`、再
`koan.UnmarshalWithConf(...)`」这个顺序假设没有任何用例覆盖（我用与 loader 相同顺序的 koanf 流程验证过
`extensionConfigs` 不会被 mapstructure
覆盖，结果是对的，但该不变量没有回归保护），`startGlobalConfigCenter` 里的
`mergeExtensionConfigs(rc.extensionConfigs, extensionConfigsFromKoanf(koan))`
同样没有覆盖。
建议补一条使用 `Load(WithBytes(...))` 的用例，断言
`dubbo.extensions.<prefix>.consumer.<k>` 的值最终出现在 `Config.Init` 可见的配置对象上。
--
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]
