Skip to content

Commit

Permalink
code cleanups; more tests
Browse files Browse the repository at this point in the history 
We clean up the code a bit, using slightly more efficient switch statements and using reflect.Values more idiomatically (they're conventionally passed by value not by pointer). Technically, changing the type of the fields in FieldConfig is an API-breaking change, but there don't seem to be any backend implementations outside of confita itself and even the ones inside confita aren't broken by the change.

Also use the correct bit size when parsing numbers because it's easy to do.

Also add some tests for error cases, which gets us to 96% coverage
from 90%.

Also use the correct module name in the `go.mod` file.
  • Loading branch information
@rogpeppe
rogpeppe committed May 9, 2019
1 parent ce4cc83 commit 3054e05
Show file tree
Hide file tree
Showing 4 changed files with 588 additions and 104 deletions.
128 changes: 66 additions & 62 deletions config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,16 @@ type Loader struct {
Tag string
}

// Unmarshaler can be implemented by backends to receive the struct directly and load values into it.
type Unmarshaler interface {
Unmarshal(ctx context.Context, to interface{}) error
}

// StructLoader can be implemented by backends to receive the parsed struct informations and load values into it.
type StructLoader interface {
LoadStruct(ctx context.Context, cfg *StructConfig) error
}

// NewLoader creates a Loader. If no backend is specified, the loader uses the environment.
func NewLoader(backends ...backend.Backend) *Loader {
l := Loader{
Expand Down Expand Up @@ -53,12 +63,12 @@ func (l *Loader) Load(ctx context.Context, to interface{}) error {

ref = ref.Elem()

s := l.parseStruct(&ref)
s := l.parseStruct(ref)
s.S = to
return l.resolve(ctx, s)
}

func (l *Loader) parseStruct(ref *reflect.Value) *StructConfig {
func (l *Loader) parseStruct(ref reflect.Value) *StructConfig {
var s StructConfig

t := ref.Type()
Expand All @@ -85,14 +95,13 @@ func (l *Loader) parseStruct(ref *reflect.Value) *StructConfig {
}

// if struct or *struct, parse recursively
switch {
case typ.Kind() == reflect.Struct:
s.Fields = append(s.Fields, l.parseStruct(&value).Fields...)
switch typ.Kind() {
case reflect.Struct:
s.Fields = append(s.Fields, l.parseStruct(value).Fields...)
continue
case typ.Kind() == reflect.Ptr:
if value.Type().Elem().Kind() == reflect.Struct && !value.IsNil() {
value = value.Elem()
s.Fields = append(s.Fields, l.parseStruct(&value).Fields...)
case reflect.Ptr:
if typ.Elem().Kind() == reflect.Struct && !value.IsNil() {
s.Fields = append(s.Fields, l.parseStruct(value.Elem()).Fields...)
continue
}
}
Expand All @@ -105,13 +114,13 @@ func (l *Loader) parseStruct(ref *reflect.Value) *StructConfig {
f := FieldConfig{
Name: field.Name,
Key: tag,
Value: &value,
Value: value,
}

// copying field content to a new value
clone := reflect.Indirect(reflect.New(f.Value.Type()))
clone.Set(*f.Value)
f.Default = &clone
clone.Set(f.Value)
f.Default = clone

if idx := strings.Index(tag, ","); idx != -1 {
f.Key = tag[:idx]
Expand Down Expand Up @@ -216,8 +225,8 @@ type StructConfig struct {
type FieldConfig struct {
Name string
Key string
Value *reflect.Value
Default *reflect.Value
Value reflect.Value
Default reflect.Value
Required bool
Backend string
}
Expand All @@ -227,47 +236,26 @@ func (f *FieldConfig) Set(data string) error {
return convert(data, f.Value)
}

func convert(data string, value *reflect.Value) error {
k := value.Kind()
t := value.Type()
var durationType = reflect.TypeOf(time.Duration(0))

switch {
case t.String() == "time.Duration":
func convert(data string, value reflect.Value) error {
t := value.Type()
if t == durationType {
d, err := time.ParseDuration(data)
if err != nil {
return err
}

value.SetInt(int64(d))
case k == reflect.Bool:
return nil
}
switch t.Kind() {
case reflect.Bool:
b, err := strconv.ParseBool(data)
if err != nil {
return err
}

value.SetBool(b)
case k >= reflect.Int && k <= reflect.Int64:
i, err := strconv.ParseInt(data, 10, 64)
if err != nil {
return err
}

value.SetInt(i)
case k >= reflect.Uint && k <= reflect.Uint64:
i, err := strconv.ParseUint(data, 10, 64)
if err != nil {
return err
}

value.SetUint(i)
case k >= reflect.Float32 && k <= reflect.Float64:
f, err := strconv.ParseFloat(data, 64)
if err != nil {
return err
}

value.SetFloat(f)
case k == reflect.Slice:
case reflect.Slice:
var err error
// create a new temporary slice to override the actual Value if it's not empty
nv := reflect.MakeSlice(value.Type(), 0, 0)
Expand All @@ -276,40 +264,56 @@ func convert(data string, value *reflect.Value) error {
// create a new Value v based on the type of the slice
v := reflect.Indirect(reflect.New(t.Elem()))
// call convert to set the current value of the slice to v
err = convert(s, &v)
err = convert(s, v)
// append v to the temporary slice
nv = reflect.Append(nv, v)
}
// Set the newly created temporary slice to the target Value
value.Set(nv)
return err

case k == reflect.String:
case reflect.String:
value.SetString(data)
case k == reflect.Ptr:
case reflect.Ptr:
n := reflect.New(value.Type().Elem())
value.Set(n)
e := n.Elem()
return convert(data, &e)
return convert(data, n.Elem())
case reflect.Int,
reflect.Int8,
reflect.Int16,
reflect.Int32,
reflect.Int64:
i, err := strconv.ParseInt(data, 10, t.Bits())
if err != nil {
return err
}

value.SetInt(i)
case reflect.Uint,
reflect.Uint8,
reflect.Uint16,
reflect.Uint32,
reflect.Uint64:
i, err := strconv.ParseUint(data, 10, t.Bits())
if err != nil {
return err
}

value.SetUint(i)
case reflect.Float32, reflect.Float64:
f, err := strconv.ParseFloat(data, t.Bits())
if err != nil {
return err
}
value.SetFloat(f)
default:
return fmt.Errorf("field type '%s' not supported", k)
return fmt.Errorf("field type '%s' not supported", t.Kind())
}

return nil
}

func isZero(v *reflect.Value) bool {
func isZero(v reflect.Value) bool {
zero := reflect.Zero(v.Type()).Interface()
current := v.Interface()
return reflect.DeepEqual(current, zero)
}

// Unmarshaler can be implemented by backends to receive the struct directly and load values into it.
type Unmarshaler interface {
Unmarshal(ctx context.Context, to interface{}) error
}

// StructLoader can be implemented by backends to receive the parsed struct informations and load values into it.
type StructLoader interface {
LoadStruct(ctx context.Context, cfg *StructConfig) error
}
99 changes: 99 additions & 0 deletions config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"math"
"regexp"
"strconv"
"testing"
"time"
Expand Down Expand Up @@ -85,6 +86,7 @@ func TestLoad(t *testing.T) {
StructPtrNil *nested
StructPtrNotNil *nested
Ignored string
unexported int `config:"ignore"`
}

var s testStruct
Expand Down Expand Up @@ -493,3 +495,100 @@ func TestSliceField(t *testing.T) {
require.Equal(t, 42, *s.Numbers[2])
})
}

var errorTests = []struct {
testName string
store store
into interface{}
expectError string
}{{
testName: "bad-duration",
store: store{
"X": "xxxx",
},
into: new(struct {
X time.Duration `config:"X"`
}),
expectError: `time: invalid duration xxxx`,
}, {
testName: "bad-bool",
store: store{
"X": "xxxx",
},
into: new(struct {
X bool `config:"X"`
}),
expectError: `strconv.ParseBool: parsing "xxxx": invalid syntax`,
}, {
testName: "bad-int",
store: store{
"X": "xxxx",
},
into: new(struct {
X int `config:"X"`
}),
expectError: `strconv.ParseInt: parsing "xxxx": invalid syntax`,
}, {
testName: "bad-uint",
store: store{
"X": "xxxx",
},
into: new(struct {
X uint `config:"X"`
}),
expectError: `strconv.ParseUint: parsing "xxxx": invalid syntax`,
}, {
testName: "out-of-range-int",
store: store{
"X": "128",
},
into: new(struct {
X int8 `config:"X"`
}),
expectError: `strconv.ParseInt: parsing "128": value out of range`,
}, {
testName: "out-of-range-uint",
store: store{
"X": "256",
},
into: new(struct {
X uint8 `config:"X"`
}),
expectError: `strconv.ParseUint: parsing "256": value out of range`,
}, {
testName: "bad-float",
store: store{
"X": "xxxx",
},
into: new(struct {
X float64 `config:"X"`
}),
expectError: `strconv.ParseFloat: parsing "xxxx": invalid syntax`,
}, {
testName: "unsupported-field-type",
store: store{
"X": "xxxx",
},
into: new(struct {
X uintptr `config:"X"`
}),
expectError: `field type 'uintptr' not supported`,
}, {
testName: "not-struct-pointer",
into: struct{}{},
expectError: `provided target must be a pointer to struct`,
}}

func TestError(t *testing.T) {
for _, test := range errorTests {
t.Run(test.testName, func(t *testing.T) {
err := confita.NewLoader(test.store).Load(context.Background(), test.into)
if err == nil {
t.Fatalf("unexpected success; into %#v", test.into)
}
if ok, err1 := regexp.MatchString("^("+test.expectError+")$", err.Error()); !ok || err1 != nil {
t.Fatalf("error mismatch; got %q want %q", err, test.expectError)
}
})
}
}
Loading

0 comments on commit 3054e05

Please sign in to comment.