Refactor to a concurrent solution and move to beatlabs organization

Signed-off-by: Sotirios Mantziaris <[email protected]>

.travis.yml

@@ -17,4 +17,5 @@ script:
   - make ci
 
 after_success:
+  - export CODECOV_TOKEN="34285c03-89af-4ff3-af0b-c846a6f43244"
   - bash <(curl -s https://codecov.io/bash)

README.md

@@ -15,9 +15,9 @@ The order is applied as it is listed above. Consul seeder and monitor are option
 
 ```go
 type Config struct {
-    Name    string  `seed:"John Doe"`
-    Age     int64   `seed:"18" env:"ENV_AGE"`
-    IsAdmin bool    `seed:"true" env:"ENV_IS_ADMIN" consul:"/config/is-admin"`
+    Name    sync.String  `seed:"John Doe"`
+    Age     sync.Int64   `seed:"18" env:"ENV_AGE"`
+    IsAdmin sync.Bool    `seed:"true" env:"ENV_IS_ADMIN" consul:"/config/is-admin"`
 }
 ```
 
@@ -27,6 +27,13 @@ The above defines the following fields:
 - Age, which will be seeded with the value `18`, and if exists, overridden with whatever value the env var `ENV_AGE` holds
 - IsAdmin, which will be seeded with the value `true`, and if exists, overridden with whatever value the env var `ENV_AGE` holds and then from consul if the consul seeder and/or watcher are provided.
 
+The fields have to be one of the types that the sync package supports in order to allow concurrent read and write to the fields. The following types are supported:
+
+- sync.String, allows for concurrent string manipulation
+- sync.Int64, allows for concurrent int64 manipulation
+- sync.Float64, allows for concurrent float64 manipulation
+- sync.Bool, allows for concurrent bool manipulation
+
 `Harvester` has a seeding phase and an optional monitoring phase.
 
 ## Seeding phase
@@ -89,6 +96,8 @@ Consul has support for versioning (`ModifyIndex`) which allows us to change the
 
 Will be github issues once we move to thebeatapp organization.
 
+- The code has a data race and cannot be used with safety.
+- The config struct should check for null fields and initialize with default pointer value
 - create examples folder with a service implementation (@d.baltas)
 - move to circle-ci
 - Error handling

change/change.go

@@ -1,6 +1,6 @@
 package change
 
-import "github.com/taxibeat/harvester/config"
+import "github.com/beatlabs/harvester/config"
 
 // Change contains all the information of a change.
 type Change struct {

change/change_test.go

@@ -3,8 +3,8 @@ package change
 import (
 	"testing"
 
+	"github.com/beatlabs/harvester/config"
 	"github.com/stretchr/testify/assert"
-	"github.com/taxibeat/harvester/config"
 )
 
 func TestChange(t *testing.T) {

config/config.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"reflect"
 	"strconv"
-	"sync"
 )
 
 // Source definition.
@@ -22,105 +21,142 @@ const (
 
 // Field definition of a config value that can change.
 type Field struct {
-	Name    string
-	Kind    reflect.Kind
-	Version uint64
-	Sources map[Source]string
+	name    string
+	tp      string
+	version uint64
+	setter  reflect.Value
+	sources map[Source]string
 }
 
-// Config manages configuration and handles updates on the values.
-type Config struct {
-	Fields []*Field
-	sync.Mutex
-	cfg reflect.Value
-}
-
-// New creates a new monitor.
-func New(cfg interface{}) (*Config, error) {
-	if cfg == nil {
-		return nil, errors.New("configuration is nil")
+// NewField constructor.
+func NewField(fld *reflect.StructField, val *reflect.Value) (*Field, error) {
+	if !isTypeSupported(fld.Type) {
+		return nil, fmt.Errorf("field %s is not supported (only types from the sync package of harvester)", fld.Name)
 	}
-	tp := reflect.TypeOf(cfg)
-	if tp.Kind() != reflect.Ptr {
-		return nil, errors.New("configuration should be a pointer type")
+	f := &Field{
+		name:    fld.Name,
+		tp:      fld.Type.Name(),
+		version: 0,
+		setter:  val.FieldByName(fld.Name).Addr().MethodByName("Set"),
+		sources: make(map[Source]string),
 	}
-	ff, err := getFields(tp.Elem())
-	if err != nil {
-		return nil, err
+	value, ok := fld.Tag.Lookup(string(SourceSeed))
+	if ok {
+		f.sources[SourceSeed] = value
 	}
-	return &Config{cfg: reflect.ValueOf(cfg).Elem(), Fields: ff}, nil
+	value, ok = fld.Tag.Lookup(string(SourceEnv))
+	if ok {
+		f.sources[SourceEnv] = value
+	}
+	value, ok = fld.Tag.Lookup(string(SourceConsul))
+	if ok {
+		f.sources[SourceConsul] = value
+	}
+	return f, nil
+}
+
+// Name getter.
+func (f *Field) Name() string {
+	return f.name
+}
+
+// Type getter.
+func (f *Field) Type() string {
+	return f.tp
+}
+
+// Sources getter.
+func (f *Field) Sources() map[Source]string {
+	return f.sources
 }
 
-// Set the value of a property of the provided config.
-func (v *Config) Set(name, value string, kind reflect.Kind) error {
-	v.Lock()
-	defer v.Unlock()
-	f := v.cfg.FieldByName(name)
-	switch kind {
-	case reflect.Bool:
-		b, err := strconv.ParseBool(value)
+// Set the value of the field.
+func (f *Field) Set(value string, version uint64) error {
+	if version != 0 && version <= f.version {
+		return fmt.Errorf("version %d is older or same as the field's %s", version, f.name)
+	}
+	var arg interface{}
+	switch f.tp {
+	case "Bool":
+		v, err := strconv.ParseBool(value)
 		if err != nil {
 			return err
 		}
-		f.SetBool(b)
-	case reflect.String:
-		f.SetString(value)
-	case reflect.Int64:
+		arg = v
+	case "String":
+		arg = value
+	case "Int64":
 		v, err := strconv.ParseInt(value, 10, 64)
 		if err != nil {
 			return err
 		}
-		f.SetInt(v)
-	case reflect.Float64:
+		arg = v
+	case "Float64":
 		v, err := strconv.ParseFloat(value, 64)
 		if err != nil {
 			return err
 		}
-		f.SetFloat(v)
-	default:
-		return fmt.Errorf("unsupported kind: %v", kind)
+		arg = v
+	}
+	rr := f.setter.Call([]reflect.Value{reflect.ValueOf(arg)})
+	if len(rr) > 0 {
+		return fmt.Errorf("the set call returned %d values: %v", len(rr), rr)
 	}
+	f.version = version
 	return nil
 }
 
-func getFields(tp reflect.Type) ([]*Field, error) {
+// Config manages configuration and handles updates on the values.
+type Config struct {
+	Fields []*Field
+}
+
+// New creates a new monitor.
+func New(cfg interface{}) (*Config, error) {
+	if cfg == nil {
+		return nil, errors.New("configuration is nil")
+	}
+	tp := reflect.TypeOf(cfg)
+	if tp.Kind() != reflect.Ptr {
+		return nil, errors.New("configuration should be a pointer type")
+	}
+	val := reflect.ValueOf(cfg).Elem()
+	ff, err := getFields(tp.Elem(), &val)
+	if err != nil {
+		return nil, err
+	}
+	return &Config{Fields: ff}, nil
+}
+
+func getFields(tp reflect.Type, val *reflect.Value) ([]*Field, error) {
 	dup := make(map[Source]string)
 	var ff []*Field
 	for i := 0; i < tp.NumField(); i++ {
-		fld := tp.Field(i)
-		kind := fld.Type.Kind()
-		if !isKindSupported(kind) {
-			return nil, fmt.Errorf("field %s is not supported(only bool, int64, float64 and string)", fld.Name)
-		}
-		f := &Field{
-			Name:    fld.Name,
-			Kind:    kind,
-			Version: 0,
-			Sources: make(map[Source]string),
-		}
-		value, ok := fld.Tag.Lookup(string(SourceSeed))
-		if ok {
-			f.Sources[SourceSeed] = value
-		}
-		value, ok = fld.Tag.Lookup(string(SourceEnv))
-		if ok {
-			f.Sources[SourceEnv] = value
+		f := tp.Field(i)
+		fld, err := NewField(&f, val)
+		if err != nil {
+			return nil, err
 		}
-		value, ok = fld.Tag.Lookup(string(SourceConsul))
+		value, ok := fld.Sources()[SourceConsul]
 		if ok {
 			if isKeyValueDuplicate(dup, SourceConsul, value) {
 				return nil, fmt.Errorf("duplicate value %s for source %s", value, SourceConsul)
 			}
-			f.Sources[SourceConsul] = value
 		}
-		ff = append(ff, f)
+		ff = append(ff, fld)
 	}
 	return ff, nil
 }
 
-func isKindSupported(kind reflect.Kind) bool {
-	switch kind {
-	case reflect.Bool, reflect.Int64, reflect.Float64, reflect.String:
+func isTypeSupported(t reflect.Type) bool {
+	if t.Kind() != reflect.Struct {
+		return false
+	}
+	if t.PkgPath() != "github.com/beatlabs/harvester/sync" {
+		return false
+	}
+	switch t.Name() {
+	case "Bool", "Int64", "Float64", "String":
 		return true
 	default:
 		return false