Skip to content

Commit 06837fd

Browse files
nanjieknanjiek
authored
migrate the ValidateRegistryIDs to internal (#3258)
* remove the config package from package client * fix(client): fail fast on invalid registry and method config * restore WithMethod in Server * fix lint of action_test * refactor: split processURL to reduce cognitive complexity * migrate the ValidateRegistryIDs to internal * fix the cmt * remove the validateRegistryIDs
1 parent 9a9c24a commit 06837fd

File tree

6 files changed

+105
-24
lines changed

6 files changed

+105
-24
lines changed

‎client/options.go‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
package client
1919

2020
import (
21-
"fmt"
2221
"strconv"
2322
"time"
2423
)
@@ -120,7 +119,7 @@ func (refOpts *ReferenceOptions) init(opts ...ReferenceOption) error {
120119
}
121120
}
122121
refConf.RegistryIDs = commonCfg.TranslateIds(refConf.RegistryIDs)
123-
if err := validateRegistryIDs(refConf.RegistryIDs, regs); err != nil {
122+
if err := internal.ValidateRegistryIDs(refConf.RegistryIDs, regs); err != nil {
124123
return err
125124
}
126125
}
@@ -560,7 +559,7 @@ func (cliOpts *ClientOptions) init(opts ...ClientOption) error {
560559
}
561560
}
562561
consumerConf.RegistryIDs = commonCfg.TranslateIds(consumerConf.RegistryIDs)
563-
if err := validateRegistryIDs(consumerConf.RegistryIDs, regs); err != nil {
562+
if err := internal.ValidateRegistryIDs(consumerConf.RegistryIDs, regs); err != nil {
564563
return err
565564
}
566565
}
@@ -966,15 +965,6 @@ func newDefaultCallOptions() *CallOptions {
966965
return &CallOptions{}
967966
}
968967

969-
func validateRegistryIDs(ids []string, regs map[string]*global.RegistryConfig) error {
970-
for _, id := range ids {
971-
if _, ok := regs[id]; !ok {
972-
return fmt.Errorf("registry id %q not found", id)
973-
}
974-
}
975-
return nil
976-
}
977-
978968
// WithCallRequestTimeout the maximum waiting time for one specific call, only works for 'tri' and 'dubbo' protocol
979969
func WithCallRequestTimeout(timeout time.Duration) CallOption {
980970
return func(opts *CallOptions) {

‎client/options_test.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1273,6 +1273,23 @@ func TestClientOptionsInitFailsOnMissingRegistryID(t *testing.T) {
12731273
assert.Contains(t, err.Error(), `registry id "missing" not found`)
12741274
}
12751275

1276+
func TestClientOptionsInitTranslatesRegistryIDs(t *testing.T) {
1277+
cliOpts := &ClientOptions{
1278+
Consumer: &global.ConsumerConfig{
1279+
RegistryIDs: []string{"r1,r2", "r2"},
1280+
},
1281+
Registries: map[string]*global.RegistryConfig{
1282+
"r1": {Protocol: "mock", Address: "127.0.0.1:2181"},
1283+
"r2": {Protocol: "mock", Address: "127.0.0.2:2181"},
1284+
},
1285+
overallReference: &global.ReferenceConfig{},
1286+
}
1287+
1288+
err := cliOpts.init()
1289+
require.NoError(t, err)
1290+
assert.Equal(t, []string{"r1", "r2"}, cliOpts.Consumer.RegistryIDs)
1291+
}
1292+
12761293
func TestReferenceOptionsInitFailsOnMissingRegistryID(t *testing.T) {
12771294
refOpts := defaultReferenceOptions()
12781295
refOpts.Registries = map[string]*global.RegistryConfig{
@@ -1284,6 +1301,18 @@ func TestReferenceOptionsInitFailsOnMissingRegistryID(t *testing.T) {
12841301
assert.Contains(t, err.Error(), `registry id "missing" not found`)
12851302
}
12861303

1304+
func TestReferenceOptionsInitTranslatesRegistryIDs(t *testing.T) {
1305+
refOpts := defaultReferenceOptions()
1306+
refOpts.Registries = map[string]*global.RegistryConfig{
1307+
"r1": {Protocol: "mock", Address: "127.0.0.1:2181"},
1308+
"r2": {Protocol: "mock", Address: "127.0.0.2:2181"},
1309+
}
1310+
1311+
err := refOpts.init(WithRegistryIDs("r1,r2", "r2"))
1312+
require.NoError(t, err)
1313+
assert.Equal(t, []string{"r1", "r2"}, refOpts.Reference.RegistryIDs)
1314+
}
1315+
12871316
func TestReferenceOptionsInitFailsOnInvalidMethodConfig(t *testing.T) {
12881317
refOpts := defaultReferenceOptions()
12891318
err := refOpts.init(WithMethod(&global.MethodConfig{

‎internal/config.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,3 +206,12 @@ func ValidateMethodConfig(method *global.MethodConfig) error {
206206

207207
return nil
208208
}
209+
210+
func ValidateRegistryIDs(ids []string, regs map[string]*global.RegistryConfig) error {
211+
for _, id := range ids {
212+
if _, ok := regs[id]; !ok {
213+
return fmt.Errorf("registry id %q not found", id)
214+
}
215+
}
216+
return nil
217+
}

‎internal/config_test.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,28 @@ func TestLoadRegistries_MissingRegistryID(t *testing.T) {
132132
assert.Contains(t, err.Error(), `registry id "missing" not found`)
133133
}
134134

135+
func TestValidateRegistryIDs(t *testing.T) {
136+
t.Run("valid ids", func(t *testing.T) {
137+
regs := map[string]*global.RegistryConfig{
138+
"r1": {Protocol: "mock", Address: "127.0.0.1:2181"},
139+
"r2": {Protocol: "mock", Address: "127.0.0.2:2181"},
140+
}
141+
142+
err := ValidateRegistryIDs([]string{"r1", "r2"}, regs)
143+
require.NoError(t, err)
144+
})
145+
146+
t.Run("missing id", func(t *testing.T) {
147+
regs := map[string]*global.RegistryConfig{
148+
"r1": {Protocol: "mock", Address: "127.0.0.1:2181"},
149+
}
150+
151+
err := ValidateRegistryIDs([]string{"r1", "missing"}, regs)
152+
require.Error(t, err)
153+
assert.Contains(t, err.Error(), `registry id "missing" not found`)
154+
})
155+
}
156+
135157
func TestToURLs_EmptyOrNA(t *testing.T) {
136158
tests := []struct {
137159
name string

‎server/options.go‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
package server
1919

2020
import (
21-
"fmt"
2221
"reflect"
2322
"strconv"
2423
"sync"
@@ -89,7 +88,7 @@ func (srvOpts *ServerOptions) init(opts ...ServerOption) error {
8988
if len(providerConf.RegistryIDs) <= 0 {
9089
providerConf.RegistryIDs = getRegistryIds(srvOpts.Registries)
9190
}
92-
if err := validateRegistryIDs(providerConf.RegistryIDs, srvOpts.Registries); err != nil {
91+
if err := internal.ValidateRegistryIDs(providerConf.RegistryIDs, srvOpts.Registries); err != nil {
9392
return err
9493
}
9594

@@ -563,7 +562,7 @@ func (svcOpts *ServiceOptions) init(srv *Server, opts ...ServiceOption) error {
563562
}
564563
if len(svc.RegistryIDs) <= 0 {
565564
svc.NotRegister = true
566-
} else if err := validateRegistryIDs(svc.RegistryIDs, svc.RCRegistriesMap); err != nil {
565+
} else if err := internal.ValidateRegistryIDs(svc.RegistryIDs, svc.RCRegistriesMap); err != nil {
567566
return err
568567
}
569568

@@ -635,15 +634,6 @@ func WithFilter(filter string) ServiceOption {
635634
}
636635
}
637636

638-
func validateRegistryIDs(ids []string, regs map[string]*global.RegistryConfig) error {
639-
for _, id := range ids {
640-
if _, ok := regs[id]; !ok {
641-
return fmt.Errorf("registry id %q not found", id)
642-
}
643-
}
644-
return nil
645-
}
646-
647637
// todo(DMwangnima): think about a more ideal configuration style
648638
func WithProtocolIDs(protocolIDs []string) ServiceOption {
649639
return func(cfg *ServiceOptions) {

‎server/options_test.go‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,19 @@ func TestServerOptionsInitFailsOnMissingRegistryID(t *testing.T) {
7575
assert.Contains(t, err.Error(), `registry id "missing" not found`)
7676
}
7777

78+
func TestServerOptionsInitTranslatesRegistryIDs(t *testing.T) {
79+
opts := defaultServerOptions()
80+
opts.Provider.RegistryIDs = []string{"r1,r2", "r2"}
81+
opts.Registries = map[string]*global.RegistryConfig{
82+
"r1": {Protocol: "mock", Address: "127.0.0.1:2181"},
83+
"r2": {Protocol: "mock", Address: "127.0.0.2:2181"},
84+
}
85+
86+
err := opts.init()
87+
require.NoError(t, err)
88+
assert.Equal(t, []string{"r1", "r2"}, opts.Provider.RegistryIDs)
89+
}
90+
7891
func TestServiceOptionsInitFailsOnMissingRegistryID(t *testing.T) {
7992
srv := &Server{
8093
cfg: &ServerOptions{
@@ -102,6 +115,34 @@ func TestServiceOptionsInitFailsOnMissingRegistryID(t *testing.T) {
102115
assert.Contains(t, err.Error(), `registry id "missing" not found`)
103116
}
104117

118+
func TestServiceOptionsInitTranslatesRegistryIDs(t *testing.T) {
119+
srv := &Server{
120+
cfg: &ServerOptions{
121+
Provider: &global.ProviderConfig{
122+
ProtocolIDs: []string{"triple"},
123+
},
124+
Registries: map[string]*global.RegistryConfig{
125+
"r1": {Protocol: "mock", Address: "127.0.0.1:2181"},
126+
"r2": {Protocol: "mock", Address: "127.0.0.2:2181"},
127+
},
128+
Protocols: map[string]*global.ProtocolConfig{
129+
"triple": {Name: "triple"},
130+
},
131+
Application: global.DefaultApplicationConfig(),
132+
},
133+
}
134+
svcOpts := defaultServiceOptions()
135+
svcOpts.Registries = srv.cfg.Registries
136+
svcOpts.Protocols = srv.cfg.Protocols
137+
svcOpts.Provider = srv.cfg.Provider
138+
139+
err := svcOpts.init(srv, func(opts *ServiceOptions) {
140+
opts.Service.RegistryIDs = []string{"r1,r2", "r2"}
141+
})
142+
require.NoError(t, err)
143+
assert.Equal(t, []string{"r1", "r2"}, svcOpts.Service.RegistryIDs)
144+
}
145+
105146
func TestServiceOptionsInitFailsOnInvalidMethodConfig(t *testing.T) {
106147
srv := &Server{
107148
cfg: &ServerOptions{

0 commit comments

Comments
 (0)