Skip to content

Commit

Permalink
fix: solve config bool field zero value bug by using pointer
Browse files Browse the repository at this point in the history 
fixes #2336
  • Loading branch information
@ev1lQuark
ev1lQuark committed Jun 28, 2023
1 parent 95accbd commit 00cb371
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 61 deletions.
4 changes: 2 additions & 2 deletions cluster/router/condition/dynamic_router.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ func generateConditions(routerConfig *config.RouterConfig) ([]*StateRouter, erro
return nil, err
}
url.AddParam(constant.RuleKey, conditionRule)
url.AddParam(constant.ForceKey, strconv.FormatBool(routerConfig.Force))
url.AddParam(constant.EnabledKey, strconv.FormatBool(routerConfig.Enabled))
url.AddParam(constant.ForceKey, strconv.FormatBool(*routerConfig.Force))
url.AddParam(constant.EnabledKey, strconv.FormatBool(*routerConfig.Enabled))
conditionRoute, err := NewConditionStateRouter(url)
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion cluster/router/tag/match.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func requestTag(invokers []protocol.Invoker, url *common.URL, invocation protoco
logger.Debugf("[tag router] filter dynamic tag address, invokers=%+v", result)
}
// returns the result directly
if cfg.Force || requestIsForce(url, invocation) {
if *cfg.Force || requestIsForce(url, invocation) {
return result
}
if len(result) != 0 {
Expand Down
7 changes: 4 additions & 3 deletions cluster/router/tag/router.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (p *PriorityRouter) Route(invokers []protocol.Invoker, url *common.URL, inv
return staticTag(invokers, url, invocation)
}
routerCfg := value.(config.RouterConfig)
if !routerCfg.Enabled || !routerCfg.Valid {
if !*routerCfg.Enabled || !*routerCfg.Valid {
return staticTag(invokers, url, invocation)
}
return dynamicTag(invokers, url, invocation, routerCfg)
Expand Down Expand Up @@ -119,9 +119,10 @@ func parseRoute(routeContent string) (*config.RouterConfig, error) {
if err != nil {
return nil, err
}
routerConfig.Valid = true
routerConfig.Valid = new(bool)
*routerConfig.Valid = true
if len(routerConfig.Tags) == 0 {
routerConfig.Valid = false
*routerConfig.Valid = false
}
return routerConfig, nil
}
86 changes: 45 additions & 41 deletions cluster/router/tag/router_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ func initUrl() {

func TestRouter(t *testing.T) {
initUrl()
trueValue := true
truePointer := &trueValue
falseValue := false
falsePointer := &falseValue
t.Run("staticEmptyTag", func(t *testing.T) {
p, err := NewTagPriorityRouter()
assert.Nil(t, err)
Expand Down Expand Up @@ -129,11 +133,11 @@ func TestRouter(t *testing.T) {
t.Run("dynamicEmptyTag_requestEmptyTag", func(t *testing.T) {
p, err := NewTagPriorityRouter()
assert.Nil(t, err)
p.routerConfigs.Store(consumerUrl.GetParam(constant.ApplicationKey, "")+constant.TagRouterRuleSuffix, config.RouterConfig{
Key: consumerUrl.GetParam(constant.ApplicationKey, "") + constant.TagRouterRuleSuffix,
Force: false,
Enabled: true,
Valid: true,
p.routerConfigs.Store(consumerUrl.Service()+constant.TagRouterRuleSuffix, config.RouterConfig{
Key: consumerUrl.Service() + constant.TagRouterRuleSuffix,
Force: falsePointer,
Enabled: truePointer,
Valid: truePointer,
})
ivk := protocol.NewBaseInvoker(url1)
ivk1 := protocol.NewBaseInvoker(url2)
Expand All @@ -149,11 +153,11 @@ func TestRouter(t *testing.T) {
t.Run("dynamicEmptyTag_requestHasTag", func(t *testing.T) {
p, err := NewTagPriorityRouter()
assert.Nil(t, err)
p.routerConfigs.Store(consumerUrl.GetParam(constant.ApplicationKey, "")+constant.TagRouterRuleSuffix, config.RouterConfig{
Key: consumerUrl.GetParam(constant.ApplicationKey, "") + constant.TagRouterRuleSuffix,
Force: false,
Enabled: true,
Valid: true,
p.routerConfigs.Store(consumerUrl.Service()+constant.TagRouterRuleSuffix, config.RouterConfig{
Key: consumerUrl.Service() + constant.TagRouterRuleSuffix,
Force: falsePointer,
Enabled: truePointer,
Valid: truePointer,
})
ivk := protocol.NewBaseInvoker(url1)
ivk1 := protocol.NewBaseInvoker(url2)
Expand All @@ -170,11 +174,11 @@ func TestRouter(t *testing.T) {
t.Run("dynamicTag_requestEmptyTag", func(t *testing.T) {
p, err := NewTagPriorityRouter()
assert.Nil(t, err)
p.routerConfigs.Store(consumerUrl.GetParam(constant.ApplicationKey, "")+constant.TagRouterRuleSuffix, config.RouterConfig{
Key: consumerUrl.GetParam(constant.ApplicationKey, "") + constant.TagRouterRuleSuffix,
Force: false,
Enabled: true,
Valid: true,
p.routerConfigs.Store(consumerUrl.Service()+constant.TagRouterRuleSuffix, config.RouterConfig{
Key: consumerUrl.Service() + constant.TagRouterRuleSuffix,
Force: falsePointer,
Enabled: truePointer,
Valid: truePointer,
Tags: []config.Tag{{
Name: "tag",
Addresses: []string{"192.168.0.3:20000"},
Expand All @@ -194,11 +198,11 @@ func TestRouter(t *testing.T) {
t.Run("dynamicTag_emptyAddress_requestHasTag", func(t *testing.T) {
p, err := NewTagPriorityRouter()
assert.Nil(t, err)
p.routerConfigs.Store(consumerUrl.GetParam(constant.ApplicationKey, "")+constant.TagRouterRuleSuffix, config.RouterConfig{
Key: consumerUrl.GetParam(constant.ApplicationKey, "") + constant.TagRouterRuleSuffix,
Force: false,
Enabled: true,
Valid: true,
p.routerConfigs.Store(consumerUrl.Service()+constant.TagRouterRuleSuffix, config.RouterConfig{
Key: consumerUrl.Service() + constant.TagRouterRuleSuffix,
Force: falsePointer,
Enabled: truePointer,
Valid: truePointer,
Tags: []config.Tag{{
Name: "tag",
}},
Expand All @@ -218,11 +222,11 @@ func TestRouter(t *testing.T) {
t.Run("dynamicTag_address_requestHasTag", func(t *testing.T) {
p, err := NewTagPriorityRouter()
assert.Nil(t, err)
p.routerConfigs.Store(consumerUrl.GetParam(constant.ApplicationKey, "")+constant.TagRouterRuleSuffix, config.RouterConfig{
Key: consumerUrl.GetParam(constant.ApplicationKey, "") + constant.TagRouterRuleSuffix,
Force: false,
Enabled: true,
Valid: true,
p.routerConfigs.Store(consumerUrl.Service()+constant.TagRouterRuleSuffix, config.RouterConfig{
Key: consumerUrl.Service() + constant.TagRouterRuleSuffix,
Force: falsePointer,
Enabled: truePointer,
Valid: truePointer,
Tags: []config.Tag{{
Name: "tag",
Addresses: []string{"192.168.0.3:20000"},
Expand All @@ -243,11 +247,11 @@ func TestRouter(t *testing.T) {
t.Run("dynamicTag_twoAddress_requestHasTag", func(t *testing.T) {
p, err := NewTagPriorityRouter()
assert.Nil(t, err)
p.routerConfigs.Store(consumerUrl.GetParam(constant.ApplicationKey, "")+constant.TagRouterRuleSuffix, config.RouterConfig{
Key: consumerUrl.GetParam(constant.ApplicationKey, "") + constant.TagRouterRuleSuffix,
Force: false,
Enabled: true,
Valid: true,
p.routerConfigs.Store(consumerUrl.Service()+constant.TagRouterRuleSuffix, config.RouterConfig{
Key: consumerUrl.Service() + constant.TagRouterRuleSuffix,
Force: falsePointer,
Enabled: truePointer,
Valid: truePointer,
Tags: []config.Tag{{
Name: "tag",
Addresses: []string{"192.168.0.1:20000", "192.168.0.3:20000"},
Expand All @@ -268,11 +272,11 @@ func TestRouter(t *testing.T) {
t.Run("dynamicTag_addressNotMatch_requestHasTag", func(t *testing.T) {
p, err := NewTagPriorityRouter()
assert.Nil(t, err)
p.routerConfigs.Store(consumerUrl.GetParam(constant.ApplicationKey, "")+constant.TagRouterRuleSuffix, config.RouterConfig{
Key: consumerUrl.GetParam(constant.ApplicationKey, "") + constant.TagRouterRuleSuffix,
Force: false,
Enabled: true,
Valid: true,
p.routerConfigs.Store(consumerUrl.Service()+constant.TagRouterRuleSuffix, config.RouterConfig{
Key: consumerUrl.Service() + constant.TagRouterRuleSuffix,
Force: falsePointer,
Enabled: truePointer,
Valid: truePointer,
Tags: []config.Tag{{
Name: "tag",
Addresses: []string{"192.168.0.4:20000"},
Expand All @@ -293,11 +297,11 @@ func TestRouter(t *testing.T) {
t.Run("dynamicTag_notValid", func(t *testing.T) {
p, err := NewTagPriorityRouter()
assert.Nil(t, err)
p.routerConfigs.Store(consumerUrl.GetParam(constant.ApplicationKey, "")+constant.TagRouterRuleSuffix, config.RouterConfig{
Key: consumerUrl.GetParam(constant.ApplicationKey, "") + constant.TagRouterRuleSuffix,
Force: false,
Enabled: true,
Valid: false,
p.routerConfigs.Store(consumerUrl.Service()+constant.TagRouterRuleSuffix, config.RouterConfig{
Key: consumerUrl.Service() + constant.TagRouterRuleSuffix,
Force: falsePointer,
Enabled: truePointer,
Valid: falsePointer,
Tags: []config.Tag{{
Name: "tag",
Addresses: []string{"192.168.0.1:20000", "192.168.0.3:20000"},
Expand Down Expand Up @@ -365,7 +369,7 @@ tags:
routerCfg := value.(config.RouterConfig)
assert.True(t, routerCfg.Key == "org.apache.dubbo.UserProvider.Test")
assert.True(t, len(routerCfg.Tags) == 2)
assert.True(t, routerCfg.Enabled)
assert.True(t, *routerCfg.Enabled)
})

t.Run("configNotValid", func(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions config/provider_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ type ProviderConfig struct {
FilterConf interface{} `yaml:"filter_conf" json:"filter_conf,omitempty" property:"filter_conf"`
ConfigType map[string]string `yaml:"config_type" json:"config_type,omitempty" property:"config_type"`
// adaptive service
AdaptiveService bool `default:"false" yaml:"adaptive-service" json:"adaptive-service" property:"adaptive-service"`
AdaptiveServiceVerbose bool `default:"false" yaml:"adaptive-service-verbose" json:"adaptive-service-verbose" property:"adaptive-service-verbose"`
AdaptiveService bool `yaml:"adaptive-service" json:"adaptive-service" property:"adaptive-service"`
AdaptiveServiceVerbose bool `yaml:"adaptive-service-verbose" json:"adaptive-service-verbose" property:"adaptive-service-verbose"`

rootConfig *RootConfig
}
Expand Down
16 changes: 8 additions & 8 deletions config/router_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ import (
type RouterConfig struct {
Scope string `validate:"required" yaml:"scope" json:"scope,omitempty" property:"scope"` // must be chosen from `service` and `application`.
Key string `validate:"required" yaml:"key" json:"key,omitempty" property:"key"` // specifies which service or application the rule body acts on.
Force bool `default:"false" yaml:"force" json:"force,omitempty" property:"force"`
Runtime bool `default:"false" yaml:"runtime" json:"runtime,omitempty" property:"runtime"`
Enabled bool `default:"true" yaml:"enabled" json:"enabled,omitempty" property:"enabled"`
Valid bool `default:"true" yaml:"valid" json:"valid,omitempty" property:"valid"`
Force *bool `default:"false" yaml:"force" json:"force,omitempty" property:"force"`
Runtime *bool `default:"false" yaml:"runtime" json:"runtime,omitempty" property:"runtime"`
Enabled *bool `default:"true" yaml:"enabled" json:"enabled,omitempty" property:"enabled"`
Valid *bool `default:"true" yaml:"valid" json:"valid,omitempty" property:"valid"`
Priority int `default:"0" yaml:"priority" json:"priority,omitempty" property:"priority"`
Conditions []string `yaml:"conditions" json:"conditions,omitempty" property:"conditions"`
Tags []Tag `yaml:"tags" json:"tags,omitempty" property:"tags"`
Expand Down Expand Up @@ -91,22 +91,22 @@ func (rcb *RouterConfigBuilder) SetKey(key string) *RouterConfigBuilder {
}

func (rcb *RouterConfigBuilder) SetForce(force bool) *RouterConfigBuilder {
rcb.routerConfig.Force = force
rcb.routerConfig.Force = &force
return rcb
}

func (rcb *RouterConfigBuilder) SetRuntime(runtime bool) *RouterConfigBuilder {
rcb.routerConfig.Runtime = runtime
rcb.routerConfig.Runtime = &runtime
return rcb
}

func (rcb *RouterConfigBuilder) SetEnabled(enabled bool) *RouterConfigBuilder {
rcb.routerConfig.Enabled = enabled
rcb.routerConfig.Enabled = &enabled
return rcb
}

func (rcb *RouterConfigBuilder) SetValid(valid bool) *RouterConfigBuilder {
rcb.routerConfig.Valid = valid
rcb.routerConfig.Valid = &valid
return rcb
}

Expand Down
2 changes: 1 addition & 1 deletion config/tracing_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type TracingConfig struct {
Name string `default:"jaeger" yaml:"name" json:"name,omitempty" property:"name"` // jaeger or zipkin(todo)
ServiceName string `yaml:"serviceName" json:"serviceName,omitempty" property:"serviceName"`
Address string `yaml:"address" json:"address,omitempty" property:"address"`
UseAgent bool `default:"false" yaml:"use-agent" json:"use-agent,omitempty" property:"use-agent"`
UseAgent *bool `default:"false" yaml:"use-agent" json:"use-agent,omitempty" property:"use-agent"`
}

// Prefix dubbo.router
Expand Down
2 changes: 1 addition & 1 deletion config/tracing_config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,5 @@ func TestTracingConfig(t *testing.T) {
assert.Nil(t, err)
assert.Equal(t, tracing.Prefix(), constant.TracingConfigPrefix)
assert.Equal(t, tracing.Name, "jaeger")
assert.Equal(t, tracing.UseAgent, false)
assert.Equal(t, *tracing.UseAgent, false)
}
2 changes: 1 addition & 1 deletion protocol/dubbo3/dubbo3_invoker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func NewDubboInvoker(url *common.URL) (*DubboInvoker, error) {
opts = append(opts, triConfig.WithJaegerConfig(
tracingConfig.Address,
tracingConfig.ServiceName,
tracingConfig.UseAgent,
*tracingConfig.UseAgent,
))
} else {
logger.Warnf("unsupported tracing name %s, now triple only support jaeger", tracingConfig.Name)
Expand Down
2 changes: 1 addition & 1 deletion protocol/dubbo3/dubbo3_protocol.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ func (dp *DubboProtocol) openServer(url *common.URL, tripleCodecType tripleConst
opts = append(opts, triConfig.WithJaegerConfig(
tracingConfig.Address,
tracingConfig.ServiceName,
tracingConfig.UseAgent,
*tracingConfig.UseAgent,
))
default:
logger.Warnf("unsupported tracing name %s, now triple only support jaeger", tracingConfig.Name)
Expand Down

0 comments on commit 00cb371

Please sign in to comment.