diff --git a/load.go b/load.go index 714f529..501de13 100644 --- a/load.go +++ b/load.go @@ -276,16 +276,24 @@ func fieldPath(typ reflect.Type, path string) string { } if field, ok := typ.FieldByName(name); ok { - if name = field.Tag.Get("conf"); len(name) == 0 { + name = field.Tag.Get("conf") + if len(name) == 0 { name = field.Name + } else if name == "_" { + name = "" } + if len(path) != 0 { path = fieldPath(field.Type, path) } } if len(path) != 0 { - name += "." + path + if len(name) == 0 { + name = path + } else { + name += "." + path + } } return name diff --git a/load_test.go b/load_test.go index 7f55d07..203281d 100644 --- a/load_test.go +++ b/load_test.go @@ -15,6 +15,15 @@ import ( ) func TestFieldPath(t *testing.T) { + + type Embedded struct { + Str string `conf:"str"` + } + + type Container struct { + Embedded `conf:"_"` + } + tests := []struct { value interface{} input string @@ -71,6 +80,20 @@ func TestFieldPath(t *testing.T) { input: "A.B", output: "a.b", }, + { + value: Container{}, + input: "Str", + output: "str", + }, + { + value: struct { + A struct { + Container `conf:"_"` + } `conf:"a"` + }{}, + input: "a.Str", + output: "a.Str", + }, } for _, test := range tests { @@ -483,3 +506,63 @@ func TestMakeEnvVars(t *testing.T) { t.Error(envVars) } } + +func TestEmbeddedStruct(t *testing.T) { + + type Child struct { + ChildField1 string + ChildField2 string + } + + type Branch struct { + Child `conf:"_"` + BranchField string + } + + type Container struct { + Branch `conf:"_"` + OtherBranch Branch + } + + testVal := Container{ + Branch: Branch{ + Child: Child{ + ChildField1: "embedded-child-1", + ChildField2: "embedded-child-2", + }, + BranchField: "embedded-branch", + }, + OtherBranch: Branch{ + Child: Child{ + ChildField1: "no-embedded-child-1", + ChildField2: "no-embedded-child-2", + }, + BranchField: "no-embedded-branch", + }, + } + + ld := Loader{ + Name: "test", + Args: []string{ + "-ChildField1", "embedded-child-1", + "-ChildField2", "embedded-child-2", + "-BranchField", "embedded-branch", + "-OtherBranch.ChildField1", "no-embedded-child-1", + "-OtherBranch.ChildField2", "no-embedded-child-2", + "-OtherBranch.BranchField", "no-embedded-branch", + }, + } + + val := reflect.New(reflect.TypeOf(testVal)) + + if _, _, err := ld.Load(val.Interface()); err != nil { + t.Error(err) + t.Log("<<<", testVal) + t.Log(">>>", val.Elem().Interface()) + return + } + + if v := val.Elem().Interface(); !reflect.DeepEqual(testVal, v) { + t.Errorf("bad value:\n<<< %#v\n>>> %#v", testVal, v) + } +} diff --git a/node.go b/node.go index 19004e2..192420f 100644 --- a/node.go +++ b/node.go @@ -178,6 +178,25 @@ func makeNodeStruct(v reflect.Value, t reflect.Type) (m Map) { m.value = v m.items = newMapItems() + populateNodeStruct(t, t.Name(), v, t, m) + + // if using the "_" notation to embed structs, it's possible that names are no longer unique. + props := make(map[string]struct{}) + for _, item := range m.Items() { + if _, ok := props[item.Name]; ok { + panic("duplicate name '" + item.Name + "' found after collapsing embedded structs in configuration: " + t.String()) + } + props[item.Name] = struct{}{} + } + + return +} + +// populateNodeStruct is the mutually recursive helper of makeNodeStruct to create the node struct with potentially +// embedded types. It will populate m with the struct fields from v. The original type and path of the current field +// are passed in order to create decent panic strings if an invalid configuration is detected. +func populateNodeStruct(originalT reflect.Type, path string, v reflect.Value, t reflect.Type, m Map) { + for i, n := 0, v.NumField(); i != n; i++ { fv := v.Field(i) ft := t.Field(i) @@ -190,6 +209,13 @@ func makeNodeStruct(v reflect.Value, t reflect.Type) (m Map) { switch name { case "-": continue + case "_": + path = path + "." + ft.Name + if ft.Type.Kind() != reflect.Struct || !ft.Anonymous { + panic("found \"_\" on invalid type at path " + path + " in configuration: " + originalT.Name()) + } + populateNodeStruct(originalT, path, fv, ft.Type, m) + continue case "": name = ft.Name } @@ -200,8 +226,6 @@ func makeNodeStruct(v reflect.Value, t reflect.Type) (m Map) { Value: makeNode(fv), }) } - - return } func makeNodeMap(v reflect.Value, t reflect.Type) (m Map) { diff --git a/node_test.go b/node_test.go index af2e94a..5774706 100644 --- a/node_test.go +++ b/node_test.go @@ -3,6 +3,7 @@ package conf import ( "fmt" "reflect" + "strings" "testing" "time" @@ -457,3 +458,105 @@ func TestNodeJSON(t *testing.T) { } }) } + +func Test_FlattenedEmbeddedStructs(t *testing.T) { + + type Smallest struct { + SmallestOne string + } + + type Small struct { + Smallest `conf:"_"` + SmallOne string + } + + type Medium struct { + Small `conf:"_"` + MediumOne string + } + + type Matroska struct { + Medium `conf:"_"` + LargeOne string + } + + m := Matroska{} + node := makeNodeStruct(reflect.ValueOf(m), reflect.TypeOf(m)) + if len(node.Items()) != 4 { + t.Errorf("expected to find four flattened fields...got %d", len(node.Items())) + } + + for _, name := range []string{"SmallestOne", "SmallOne", "MediumOne", "LargeOne"} { + f := node.Item(name) + if f == nil { + t.Errorf("flattened field %s is missing", name) + } + if f.Kind() != ScalarNode { + t.Errorf("flattened field %s should have been scalar but was %d", name, f.Kind()) + } + } +} + +func Test_InvalidFlattenedEmbeddedStructs(t *testing.T) { + + type Thing1 struct { + Stuff string + } + + type Thing2 struct { + Stuff string + } + + type ConflictingName struct { + Thing1 `conf:"_"` + Thing2 `conf:"_"` + } + + type EmbedPrimitive struct { + Str string `conf:"_"` + } + + type EmbedNamedStruct struct { + Thing Thing1 `conf:"_"` + } + + tests := []struct { + val interface{} + errFragments []string + } { + { + val: ConflictingName{}, + errFragments: []string{"'Stuff'", "duplicate"}, + }, + { + val: EmbedPrimitive{}, + errFragments: []string{"\"_\"", "at path EmbedPrimitive.Str"}, + }, + { + val: EmbedNamedStruct{}, + errFragments: []string{"\"_\"", "at path EmbedNamedStruct.Thing"}, + }, + } + + for _, tt := range tests { + t.Run(reflect.TypeOf(tt.val).Name(), func(t *testing.T) { + defer func() { + recovered := recover() + msg, ok := recovered.(string) + if !ok { + t.Errorf("expected a string to be recovered...got %v", recovered) + } + + // NOTE : ensure that the type name is included in the message! + for _, frag := range append(tt.errFragments, reflect.TypeOf(tt.val).Name()) { + if !strings.Contains(msg, frag) { + t.Errorf("message should have contained fragment \"%s\": %s", frag, msg) + } + } + }() + + makeNodeStruct(reflect.ValueOf(tt.val), reflect.TypeOf(tt.val)) + t.Error("test should have paniced") + }) + } +}