configstruct: make nested config structs work

fs/config/configstruct/configstruct.go

@@ -63,9 +63,9 @@ func StringToInterface(def interface{}, in string) (newValue interface{}, err er
 
 // Item describes a single entry in the options structure
 type Item struct {
-	Name  string // snake_case
+	Name  string            // snake_case
-	Field string // CamelCase
+	Field string            // CamelCase
-	Num   int    // number of the field in the struct
+	Set   func(interface{}) // set this field
 	Value interface{}
 }
 
@@ -76,6 +76,10 @@ type Item struct {
 //
 // The config_name is looked up in a struct tag called "config" or if
 // not found is the field name converted from CamelCase to snake_case.
+//
+// Nested structs are looked up too. If the parent struct has a struct
+// tag, this will be used as a prefix for the values in the sub
+// struct, otherwise they will be embedded as they are.
 func Items(opt interface{}) (items []Item, err error) {
 	def := reflect.ValueOf(opt)
 	if def.Kind() != reflect.Ptr {
@@ -87,19 +91,38 @@ func Items(opt interface{}) (items []Item, err error) {
 	}
 	defType := def.Type()
 	for i := 0; i < def.NumField(); i++ {
-		field := defType.Field(i)
+		field := def.Field(i)
-		fieldName := field.Name
+		fieldType := defType.Field(i)
-		configName, ok := field.Tag.Lookup("config")
+		fieldName := fieldType.Name
-		if !ok {
+		configName, hasTag := fieldType.Tag.Lookup("config")
+		if !hasTag {
 			configName = camelToSnake(fieldName)
 		}
-		defaultItem := Item{
+		valuePtr := field.Addr().Interface()                   // pointer to the value as an interface
-			Name:  configName,
+		_, canSet := valuePtr.(interface{ Set(string) error }) // can we set this with the Option Set protocol
-			Field: fieldName,
+		// If we have a nested struct that isn't a config item then recurse
-			Num:   i,
+		if fieldType.Type.Kind() == reflect.Struct && !canSet {
-			Value: def.Field(i).Interface(),
+			newItems, err := Items(valuePtr)
+			if err != nil {
+				return nil, fmt.Errorf("error parsing field %q: %w", fieldName, err)
+			}
+			for _, newItem := range newItems {
+				if hasTag {
+					newItem.Name = configName + "_" + newItem.Name
+				}
+				items = append(items, newItem)
+			}
+		} else {
+			defaultItem := Item{
+				Name:  configName,
+				Field: fieldName,
+				Set: func(newValue interface{}) {
+					field.Set(reflect.ValueOf(newValue))
+				},
+				Value: field.Interface(),
+			}
+			items = append(items, defaultItem)
 		}
-		items = append(items, defaultItem)
 	}
 	return items, nil
 }
@@ -122,7 +145,6 @@ func Set(config configmap.Getter, opt interface{}) (err error) {
 	if err != nil {
 		return err
 	}
-	defStruct := reflect.ValueOf(opt).Elem()
 	for _, defaultItem := range defaultItems {
 		newValue := defaultItem.Value
 		if configValue, ok := config.Get(defaultItem.Name); ok {
@@ -139,7 +161,7 @@ func Set(config configmap.Getter, opt interface{}) (err error) {
 				newValue = newNewValue
 			}
 		}
-		defStruct.Field(defaultItem.Num).Set(reflect.ValueOf(newValue))
+		defaultItem.Set(newValue)
 	}
 	return nil
 }

fs/config/configstruct/configstruct_test.go

@@ -11,12 +11,12 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-type conf struct {
+type Conf struct {
 	A string
 	B string
 }
 
-type conf2 struct {
+type Conf2 struct {
 	PotatoPie      string `config:"spud_pie"`
 	BeanStew       bool
 	RaisinRoll     int
@@ -26,6 +26,14 @@ type conf2 struct {
 	TotalWeight    fs.SizeSuffix
 }
 
+type ConfNested struct {
+	Conf             // embedded struct with no tag
+	Sub1 Conf        `config:"sub"` // member struct with tag
+	Sub2 Conf2       // member struct without tag
+	C    string      // normal item
+	D    fs.Tristate // an embedded struct which we don't want to recurse
+}
+
 func TestItemsError(t *testing.T) {
 	_, err := configstruct.Items(nil)
 	assert.EqualError(t, err, "argument must be a pointer")
@@ -33,8 +41,18 @@ func TestItemsError(t *testing.T) {
 	assert.EqualError(t, err, "argument must be a pointer to a struct")
 }
 
+// Check each item has a Set function pointer then clear it for the assert.Equal
+func cleanItems(t *testing.T, items []configstruct.Item) []configstruct.Item {
+	for i := range items {
+		item := &items[i]
+		assert.NotNil(t, item.Set)
+		item.Set = nil
+	}
+	return items
+}
+
 func TestItems(t *testing.T) {
-	in := &conf2{
+	in := &Conf2{
 		PotatoPie:      "yum",
 		BeanStew:       true,
 		RaisinRoll:     42,
@@ -46,22 +64,64 @@ func TestItems(t *testing.T) {
 	got, err := configstruct.Items(in)
 	require.NoError(t, err)
 	want := []configstruct.Item{
-		{Name: "spud_pie", Field: "PotatoPie", Num: 0, Value: string("yum")},
+		{Name: "spud_pie", Field: "PotatoPie", Value: string("yum")},
-		{Name: "bean_stew", Field: "BeanStew", Num: 1, Value: true},
+		{Name: "bean_stew", Field: "BeanStew", Value: true},
-		{Name: "raisin_roll", Field: "RaisinRoll", Num: 2, Value: int(42)},
+		{Name: "raisin_roll", Field: "RaisinRoll", Value: int(42)},
-		{Name: "sausage_on_stick", Field: "SausageOnStick", Num: 3, Value: int64(101)},
+		{Name: "sausage_on_stick", Field: "SausageOnStick", Value: int64(101)},
-		{Name: "forbidden_fruit", Field: "ForbiddenFruit", Num: 4, Value: uint(6)},
+		{Name: "forbidden_fruit", Field: "ForbiddenFruit", Value: uint(6)},
-		{Name: "cooking_time", Field: "CookingTime", Num: 5, Value: fs.Duration(42 * time.Second)},
+		{Name: "cooking_time", Field: "CookingTime", Value: fs.Duration(42 * time.Second)},
-		{Name: "total_weight", Field: "TotalWeight", Num: 6, Value: fs.SizeSuffix(17 << 20)},
+		{Name: "total_weight", Field: "TotalWeight", Value: fs.SizeSuffix(17 << 20)},
 	}
-	assert.Equal(t, want, got)
+	assert.Equal(t, want, cleanItems(t, got))
+}
+
+func TestItemsNested(t *testing.T) {
+	in := ConfNested{
+		Conf: Conf{
+			A: "1",
+			B: "2",
+		},
+		Sub1: Conf{
+			A: "3",
+			B: "4",
+		},
+		Sub2: Conf2{
+			PotatoPie:      "yum",
+			BeanStew:       true,
+			RaisinRoll:     42,
+			SausageOnStick: 101,
+			ForbiddenFruit: 6,
+			CookingTime:    fs.Duration(42 * time.Second),
+			TotalWeight:    fs.SizeSuffix(17 << 20),
+		},
+		C: "normal",
+		D: fs.Tristate{Value: true, Valid: true},
+	}
+	got, err := configstruct.Items(&in)
+	require.NoError(t, err)
+	want := []configstruct.Item{
+		{Name: "a", Field: "A", Value: string("1")},
+		{Name: "b", Field: "B", Value: string("2")},
+		{Name: "sub_a", Field: "A", Value: string("3")},
+		{Name: "sub_b", Field: "B", Value: string("4")},
+		{Name: "spud_pie", Field: "PotatoPie", Value: string("yum")},
+		{Name: "bean_stew", Field: "BeanStew", Value: true},
+		{Name: "raisin_roll", Field: "RaisinRoll", Value: int(42)},
+		{Name: "sausage_on_stick", Field: "SausageOnStick", Value: int64(101)},
+		{Name: "forbidden_fruit", Field: "ForbiddenFruit", Value: uint(6)},
+		{Name: "cooking_time", Field: "CookingTime", Value: fs.Duration(42 * time.Second)},
+		{Name: "total_weight", Field: "TotalWeight", Value: fs.SizeSuffix(17 << 20)},
+		{Name: "c", Field: "C", Value: string("normal")},
+		{Name: "d", Field: "D", Value: fs.Tristate{Value: true, Valid: true}},
+	}
+	assert.Equal(t, want, cleanItems(t, got))
 }
 
 func TestSetBasics(t *testing.T) {
-	c := &conf{A: "one", B: "two"}
+	c := &Conf{A: "one", B: "two"}
 	err := configstruct.Set(configMap{}, c)
 	require.NoError(t, err)
-	assert.Equal(t, &conf{A: "one", B: "two"}, c)
+	assert.Equal(t, &Conf{A: "one", B: "two"}, c)
 }
 
 // a simple configmap.Getter for testing
@@ -74,17 +134,17 @@ func (c configMap) Get(key string) (value string, ok bool) {
 }
 
 func TestSetMore(t *testing.T) {
-	c := &conf{A: "one", B: "two"}
+	c := &Conf{A: "one", B: "two"}
 	m := configMap{
 		"a": "ONE",
 	}
 	err := configstruct.Set(m, c)
 	require.NoError(t, err)
-	assert.Equal(t, &conf{A: "ONE", B: "two"}, c)
+	assert.Equal(t, &Conf{A: "ONE", B: "two"}, c)
 }
 
 func TestSetFull(t *testing.T) {
-	in := &conf2{
+	in := &Conf2{
 		PotatoPie:      "yum",
 		BeanStew:       true,
 		RaisinRoll:     42,
@@ -102,7 +162,7 @@ func TestSetFull(t *testing.T) {
 		"cooking_time":     "43s",
 		"total_weight":     "18M",
 	}
-	want := &conf2{
+	want := &Conf2{
 		PotatoPie:      "YUM",
 		BeanStew:       false,
 		RaisinRoll:     43,