From 1028e38e62989736f62f71f55c7faf753155d009 Mon Sep 17 00:00:00 2001
From: Evgeniy Kulikov <im@kulikov.im>
Date: Tue, 17 Sep 2019 18:12:47 +0300
Subject: [PATCH] Fixes around worker locker

- test coverage
- worker by default not required locker (settings)
- there's interface to apply settings for custom locker (redis, etc)

Signed-off-by: Evgeniy Kulikov <im@kulikov.im>
---
 workers/errors.go       |  4 ++
 workers/workers.go      | 21 +++++++++-
 workers/workers_test.go | 92 ++++++++++++++++++++++++++++++++++++++---
 3 files changed, 109 insertions(+), 8 deletions(-)

diff --git a/workers/errors.go b/workers/errors.go
index 331a8f7..e4e0b12 100644
--- a/workers/errors.go
+++ b/workers/errors.go
@@ -13,6 +13,10 @@ const (
 	// ErrEmptyWorkers when workers not passed to params
 	ErrEmptyWorkers = Error("empty workers")
 
+	// ErrEmptyLocker when locker required,
+	// but not passed to params
+	ErrEmptyLocker = Error("empty locker")
+
 	// ErrEmptyJob when worker job is nil
 	ErrEmptyJob = Error("empty job")
 )
diff --git a/workers/workers.go b/workers/workers.go
index c37b31f..07a84df 100644
--- a/workers/workers.go
+++ b/workers/workers.go
@@ -20,6 +20,11 @@ type (
 		Locker worker.Locker `optional:"true"`
 	}
 
+	// LockerSettings creates copy of locker and applies settings
+	LockerSettings interface {
+		Apply(key string, v *viper.Viper) (worker.Locker, error)
+	}
+
 	options struct {
 		Name   string
 		Job    worker.Job
@@ -103,8 +108,20 @@ func workerByConfig(opts options) (*worker.Worker, error) {
 	if opts.Viper.IsSet(key + ".immediately") {
 		w = w.SetImmediately(opts.Viper.GetBool(key + ".immediately"))
 	}
-	if opts.Locker != nil {
-		w = w.WithLock(opts.Locker)
+
+	if opts.Viper.IsSet(key + ".lock") {
+		if opts.Locker == nil {
+			return nil, errors.Wrap(ErrEmptyLocker, key)
+		} else if l, ok := opts.Locker.(LockerSettings); ok {
+			locker, err := l.Apply(key, opts.Viper)
+			if err != nil {
+				return nil, err
+			}
+
+			w = w.WithLock(locker)
+		} else {
+			w = w.WithLock(opts.Locker)
+		}
 	}
 
 	return w, nil
diff --git a/workers/workers_test.go b/workers/workers_test.go
index 6e75625..842173a 100644
--- a/workers/workers_test.go
+++ b/workers/workers_test.go
@@ -14,7 +14,14 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-type config = map[string]interface{}
+type (
+	config = map[string]interface{}
+
+	testLocker   struct{ locked int32 }
+	customLocker struct{ testLocker }
+)
+
+const errFailToApplyLockerSettings = Error("fail to apply locker settings")
 
 func mockedViper(cfg config) *viper.Viper {
 	v := viper.New()
@@ -28,18 +35,21 @@ func mockedViper(cfg config) *viper.Viper {
 	return v
 }
 
-type customLocker struct {
-	locked int32
+func (c *customLocker) Apply(key string, v *viper.Viper) (worker.Locker, error) {
+	if v.GetBool(key + ".lock.fail") {
+		return c, errFailToApplyLockerSettings
+	}
+	return c, nil
 }
 
-func (c *customLocker) Lock() error {
+func (c *testLocker) Lock() error {
 	if atomic.CompareAndSwapInt32(&c.locked, 0, 1) {
 		return nil
 	}
 	return errors.New("locked")
 }
 
-func (c *customLocker) Unlock() {
+func (c *testLocker) Unlock() {
 	atomic.StoreInt32(&c.locked, 0)
 }
 
@@ -135,7 +145,26 @@ func Test_workerByConfig(t *testing.T) {
 			Viper: mockedViper(config{}),
 		}},
 
-		{name: "good case",
+		{
+			name: "missing locker",
+			err:  ErrEmptyLocker,
+			args: options{
+				Name: "test",
+				Job:  nopJob,
+				Viper: mockedViper(config{
+					"disabled":    false,
+					"timer":       time.Millisecond,
+					"ticker":      time.Millisecond,
+					"cron":        "0 1 * * * *",
+					"immediately": true,
+					"lock":        true,
+				}),
+			},
+		},
+
+		{
+			name: "error when apply locker settings",
+			err:  errFailToApplyLockerSettings,
 			args: options{
 				Name:   "test",
 				Job:    nopJob,
@@ -146,6 +175,57 @@ func Test_workerByConfig(t *testing.T) {
 					"ticker":      time.Millisecond,
 					"cron":        "0 1 * * * *",
 					"immediately": true,
+					"lock":        true,
+					"lock.fail":   true,
+				}),
+			},
+		},
+
+		{
+			name: "good case",
+			args: options{
+				Name: "test",
+				Job:  nopJob,
+				Viper: mockedViper(config{
+					"disabled":    false,
+					"timer":       time.Millisecond,
+					"ticker":      time.Millisecond,
+					"cron":        "0 1 * * * *",
+					"immediately": true,
+				}),
+			},
+		},
+
+		{
+			name: "good case with custom locker",
+			args: options{
+				Name:   "test",
+				Job:    nopJob,
+				Locker: &customLocker{},
+				Viper: mockedViper(config{
+					"disabled":    false,
+					"timer":       time.Millisecond,
+					"ticker":      time.Millisecond,
+					"cron":        "0 1 * * * *",
+					"immediately": true,
+					"lock":        true,
+				}),
+			},
+		},
+
+		{
+			name: "good case with simple locker",
+			args: options{
+				Name:   "test",
+				Job:    nopJob,
+				Locker: &testLocker{},
+				Viper: mockedViper(config{
+					"disabled":    false,
+					"timer":       time.Millisecond,
+					"ticker":      time.Millisecond,
+					"cron":        "0 1 * * * *",
+					"immediately": true,
+					"lock":        true,
 				}),
 			},
 		},