Skip to content

Commit

Permalink
fix: feedback of author
Browse files Browse the repository at this point in the history 
Signed-off-by: hlts2 <[email protected]>
  • Loading branch information
@hlts2 @actions-user
hlts2 authored and actions-user committed Jul 28, 2020
1 parent 5ef3d3e commit cc032ed
Show file tree
Hide file tree
Showing 4 changed files with 334 additions and 339 deletions.
8 changes: 0 additions & 8 deletions internal/db/kvs/redis/option.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ var (
defaultOpts = []Option{
WithInitialPingDuration("30ms"),
WithInitialPingTimeLimit("5m"),
WithPing(true),
}
)

Expand Down Expand Up @@ -323,10 +322,3 @@ func WithInitialPingDuration(dur string) Option {
return nil
}
}

func WithPing(enabled bool) Option {
return func(r *redisClient) error {
r.pingEnabled = enabled
return nil
}
}
114 changes: 0 additions & 114 deletions internal/db/kvs/redis/option_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3077,117 +3077,3 @@ func TestWithInitialPingDuration(t *testing.T) {
})
}
}

func TestWithPingFlag(t *testing.T) {
// Change interface type to the type of object you are testing
type T = interface{}
type args struct {
flag bool
}
type want struct {
obj *T
// Uncomment this line if the option returns an error, otherwise delete it
// err error
}
type test struct {
name string
args args
want want
// Use the first line if the option returns an error. otherwise use the second line
// checkFunc func(want, *T, error) error
// checkFunc func(want, *T) error
beforeFunc func(args)
afterFunc func(args)
}

// Uncomment this block if the option returns an error, otherwise delete it
/*
defaultCheckFunc := func(w want, obj *T, err error) error {
if !errors.Is(err, w.err) {
return errors.Errorf("got error = %v, want %v", err, w.err)
}
if !reflect.DeepEqual(obj, w.obj) {
return errors.Errorf("got = %v, want %v", obj, w.obj)
}
return nil
}
*/

// Uncomment this block if the option do not returns an error, otherwise delete it
/*
defaultCheckFunc := func(w want, obj *T) error {
if !reflect.DeepEqual(obj, w.obj) {
return errors.Errorf("got = %v, want %v", obj, w.obj)
}
return nil
}
*/

tests := []test{
// TODO test cases
/*
{
name: "test_case_1",
args: args {
flag: false,
},
want: want {
obj: new(T),
},
},
*/

// TODO test cases
/*
func() test {
return test {
name: "test_case_2",
args: args {
flag: false,
},
want: want {
obj: new(T),
},
}
}(),
*/
}

for _, test := range tests {
t.Run(test.name, func(tt *testing.T) {
defer goleak.VerifyNone(tt)
if test.beforeFunc != nil {
test.beforeFunc(test.args)
}
if test.afterFunc != nil {
defer test.afterFunc(test.args)
}

// Uncomment this block if the option returns an error, otherwise delete it
/*
if test.checkFunc == nil {
test.checkFunc = defaultCheckFunc
}
got := WithPingFlag(test.args.flag)
obj := new(T)
if err := test.checkFunc(test.want, obj, got(obj)); err != nil {
tt.Errorf("error = %v", err)
}
*/

// Uncomment this block if the option do not return an error, otherwise delete it
/*
if test.checkFunc == nil {
test.checkFunc = defaultCheckFunc
}
got := WithPingFlag(test.args.flag)
obj := new(T)
got(obj)
if err := test.checkFunc(test.want, obj); err != nil {
tt.Errorf("error = %v", err)
}
*/
})
}
}
117 changes: 59 additions & 58 deletions internal/db/kvs/redis/redis.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,7 @@ type redisClient struct {
routeRandomly bool
tlsConfig *tls.Config
writeTimeout time.Duration

client Redis
pingEnabled bool
client Redis
}

// New returns Redis implementation if no error occurs.
Expand All @@ -97,82 +95,85 @@ func New(ctx context.Context, opts ...Option) (rc Redis, err error) {
}
}

switch len(r.addrs) {
r, err = r.newRedisClient(ctx)
if err != nil {
return nil, err
}

return r.ping(ctx)
}

func (rc *redisClient) newRedisClient(ctx context.Context) (*redisClient, error) {
switch len(rc.addrs) {
case 0:
return nil, errors.ErrRedisAddrsNotFound
case 1:
if len(r.addrs[0]) == 0 {
if len(rc.addrs[0]) == 0 {
return nil, errors.ErrRedisAddrsNotFound
}
r.client = redis.NewClient(&redis.Options{
Addr: r.addrs[0],
Password: r.password,
Dialer: r.dialer,
OnConnect: r.onConnect,
DB: r.db,
MaxRetries: r.maxRetries,
MinRetryBackoff: r.minRetryBackoff,
MaxRetryBackoff: r.maxRetryBackoff,
DialTimeout: r.dialTimeout,
ReadTimeout: r.readTimeout,
WriteTimeout: r.writeTimeout,
PoolSize: r.poolSize,
MinIdleConns: r.minIdleConns,
MaxConnAge: r.maxConnAge,
PoolTimeout: r.poolTimeout,
IdleTimeout: r.idleTimeout,
IdleCheckFrequency: r.idleCheckFrequency,
TLSConfig: r.tlsConfig,
rc.client = redis.NewClient(&redis.Options{
Addr: rc.addrs[0],
Password: rc.password,
Dialer: rc.dialer,
OnConnect: rc.onConnect,
DB: rc.db,
MaxRetries: rc.maxRetries,
MinRetryBackoff: rc.minRetryBackoff,
MaxRetryBackoff: rc.maxRetryBackoff,
DialTimeout: rc.dialTimeout,
ReadTimeout: rc.readTimeout,
WriteTimeout: rc.writeTimeout,
PoolSize: rc.poolSize,
MinIdleConns: rc.minIdleConns,
MaxConnAge: rc.maxConnAge,
PoolTimeout: rc.poolTimeout,
IdleTimeout: rc.idleTimeout,
IdleCheckFrequency: rc.idleCheckFrequency,
TLSConfig: rc.tlsConfig,
})
default:
r.client = redis.NewClusterClient(&redis.ClusterOptions{
Addrs: r.addrs,
Dialer: r.dialer,
MaxRedirects: r.maxRedirects,
ReadOnly: r.readOnly,
RouteByLatency: r.routeByLatency,
RouteRandomly: r.routeRandomly,
ClusterSlots: r.clusterSlots,
OnNewNode: r.onNewNode,
OnConnect: r.onConnect,
Password: r.password,
MaxRetries: r.maxRetries,
MinRetryBackoff: r.minRetryBackoff,
MaxRetryBackoff: r.maxRetryBackoff,
DialTimeout: r.dialTimeout,
ReadTimeout: r.readTimeout,
WriteTimeout: r.writeTimeout,
PoolSize: r.poolSize,
MinIdleConns: r.minIdleConns,
MaxConnAge: r.maxConnAge,
PoolTimeout: r.poolTimeout,
IdleTimeout: r.idleTimeout,
IdleCheckFrequency: r.idleCheckFrequency,
TLSConfig: r.tlsConfig,
rc.client = redis.NewClusterClient(&redis.ClusterOptions{
Addrs: rc.addrs,
Dialer: rc.dialer,
MaxRedirects: rc.maxRedirects,
ReadOnly: rc.readOnly,
RouteByLatency: rc.routeByLatency,
RouteRandomly: rc.routeRandomly,
ClusterSlots: rc.clusterSlots,
OnNewNode: rc.onNewNode,
OnConnect: rc.onConnect,
Password: rc.password,
MaxRetries: rc.maxRetries,
MinRetryBackoff: rc.minRetryBackoff,
MaxRetryBackoff: rc.maxRetryBackoff,
DialTimeout: rc.dialTimeout,
ReadTimeout: rc.readTimeout,
WriteTimeout: rc.writeTimeout,
PoolSize: rc.poolSize,
MinIdleConns: rc.minIdleConns,
MaxConnAge: rc.maxConnAge,
PoolTimeout: rc.poolTimeout,
IdleTimeout: rc.idleTimeout,
IdleCheckFrequency: rc.idleCheckFrequency,
TLSConfig: rc.tlsConfig,
}).WithContext(ctx)
}

if r.pingEnabled {
if err = r.ping(ctx); err != nil {
return nil, err
}
}

return r.client, nil
return rc, nil
}

func (rc *redisClient) ping(ctx context.Context) (err error) {
func (rc *redisClient) ping(ctx context.Context) (r Redis, err error) {
pctx, cancel := context.WithTimeout(ctx, rc.initialPingTimeLimit)
defer cancel()
tick := time.NewTicker(rc.initialPingDuration)
for {
select {
case <-pctx.Done():
return errors.Wrap(errors.Wrap(err, errors.ErrRedisConnectionPingFailed.Error()), pctx.Err().Error())
return nil, errors.Wrap(errors.Wrap(err, errors.ErrRedisConnectionPingFailed.Error()), pctx.Err().Error())
case <-tick.C:
err = rc.client.Ping().Err()
if err == nil {
return nil
return rc.client, nil
}
log.Error(err)
}
Expand Down
Loading

0 comments on commit cc032ed

Please sign in to comment.