From 35fa7801c6aa83560a79d132c13e16741fd1e7b2 Mon Sep 17 00:00:00 2001 From: Jabar Asadi Date: Mon, 23 Feb 2026 16:26:33 +0100 Subject: [PATCH 1/2] support nested struct binding in slices and maps --- binding.go | 151 ++++++++++++++++++++++++++++++++++++++++++++++++- loader.go | 111 +++++++++++++++++++++++++++++++++++- loader_test.go | 121 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 379 insertions(+), 4 deletions(-) diff --git a/binding.go b/binding.go index e8f6513..14936bd 100644 --- a/binding.go +++ b/binding.go @@ -387,17 +387,91 @@ func convertValue(rawValue any, targetType reflect.Type) (any, error) { return val, nil case reflect.Slice: - // Handle []string + // Preserve the existing []string convenience behavior (comma-separated strings). if targetType.Elem().Kind() == reflect.String { return parseStringSlice(rawValue) } - return nil, fmt.Errorf("unsupported slice type: %s", targetType) + + rawValueRef := reflect.ValueOf(rawValue) + if rawValueRef.Kind() != reflect.Slice && rawValueRef.Kind() != reflect.Array { + return nil, fmt.Errorf("cannot convert %T to %s", rawValue, targetType) + } + + result := reflect.MakeSlice(targetType, rawValueRef.Len(), rawValueRef.Len()) + for i := 0; i < rawValueRef.Len(); i++ { + elem, err := convertCollectionElement(rawValueRef.Index(i).Interface(), targetType.Elem()) + if err != nil { + return nil, fmt.Errorf("cannot convert slice element %d: %w", i, err) + } + result.Index(i).Set(reflect.ValueOf(elem)) + } + return result.Interface(), nil + + case reflect.Map: + if targetType.Key().Kind() != reflect.String { + return nil, fmt.Errorf("unsupported map key type: %s", targetType.Key()) + } + + rawValueRef := reflect.ValueOf(rawValue) + if rawValueRef.Kind() != reflect.Map { + return nil, fmt.Errorf("cannot convert %T to %s", rawValue, targetType) + } + + result := reflect.MakeMapWithSize(targetType, rawValueRef.Len()) + for _, rawKey := range rawValueRef.MapKeys() { + if rawKey.Kind() != reflect.String { + return nil, fmt.Errorf("cannot convert map key type %s to string", rawKey.Type()) + } + + elem, err := convertCollectionElement(rawValueRef.MapIndex(rawKey).Interface(), targetType.Elem()) + if err != nil { + return nil, fmt.Errorf("cannot convert map value for key %q: %w", rawKey.String(), err) + } + result.SetMapIndex(reflect.ValueOf(rawKey.String()).Convert(targetType.Key()), reflect.ValueOf(elem)) + } + return result.Interface(), nil default: return nil, fmt.Errorf("unsupported target type: %s", targetType) } } +func convertCollectionElement(rawValue any, targetType reflect.Type) (any, error) { + // Collection elements/values that are structs need real binding, not "return raw map as-is". + if rawValue != nil && + targetType.Kind() == reflect.Struct && + targetType != timeType && + targetType != durationType && + !isOptionalType(targetType) { + return convertStructValue(rawValue, targetType) + } + + return convertValue(rawValue, targetType) +} + +func convertStructValue(rawValue any, targetType reflect.Type) (any, error) { + rawValueRef := reflect.ValueOf(rawValue) + if !rawValueRef.IsValid() || rawValueRef.Kind() != reflect.Map { + return convertValue(rawValue, targetType) + } + + nestedData := make(map[string]mergedEntry, rawValueRef.Len()) + for _, rawKey := range rawValueRef.MapKeys() { + if rawKey.Kind() != reflect.String { + return nil, fmt.Errorf("cannot bind nested struct %s with non-string key type %s", targetType, rawKey.Type()) + } + nestedData[strings.ToLower(rawKey.String())] = mergedEntry{value: rawValueRef.MapIndex(rawKey).Interface()} + } + + nestedTarget := reflect.New(targetType).Elem() + fieldErrors := bindStruct(nestedTarget, nestedData, nil, "", "") + if len(fieldErrors) > 0 { + return nil, fmt.Errorf("cannot bind nested struct %s at %s: %s", targetType, fieldErrors[0].FieldPath, fieldErrors[0].Message) + } + + return nestedTarget.Interface(), nil +} + // parseBool parses a boolean value from a string. // Accepts: "true", "false", "1", "0", "yes", "no" (case-insensitive) func parseBool(s string) (bool, error) { @@ -443,6 +517,70 @@ func parseStringSlice(rawValue any) ([]string, error) { } } +func synthesizeNestedMapEntry(data map[string]mergedEntry, keyPath string) (mergedEntry, bool) { + prefix := keyPath + "." + nested := make(map[string]any) + + var sourceName string + var sourceKey string + found := false + mixedSources := false + + for dataKey, entry := range data { + if !strings.HasPrefix(dataKey, prefix) { + continue + } + + parts := strings.Split(strings.TrimPrefix(dataKey, prefix), ".") + if len(parts) == 0 || parts[0] == "" { + continue + } + + setNestedMapValue(nested, parts, entry.value) + if !found { + sourceName = entry.sourceName + sourceKey = entry.sourceKey + found = true + continue + } + + if sourceName != entry.sourceName || sourceKey != entry.sourceKey { + mixedSources = true + } + } + + if !found { + return mergedEntry{}, false + } + + if mixedSources { + // A single field-level provenance source would be misleading for a synthesized mixed-source map. + sourceName = "" + sourceKey = "" + } + + return mergedEntry{ + value: nested, + sourceName: sourceName, + sourceKey: sourceKey, + }, true +} + +func setNestedMapValue(dst map[string]any, parts []string, value any) { + if len(parts) == 1 { + dst[parts[0]] = value + return + } + + child, ok := dst[parts[0]].(map[string]any) + if !ok { + child = make(map[string]any) + dst[parts[0]] = child + } + + setNestedMapValue(child, parts[1:], value) +} + // mergedEntry represents a configuration value with its source information. type mergedEntry struct { value any @@ -564,6 +702,13 @@ func bindScalarField( var rawValue any var sourceName string + if !found && fieldValue.Kind() == reflect.Map { + if synthesizedEntry, ok := synthesizeNestedMapEntry(data, keyPath); ok { + entry = synthesizedEntry + found = true + } + } + if found { rawValue = entry.value sourceName = entry.sourceName @@ -599,7 +744,7 @@ func bindScalarField( if fieldValue.CanSet() { fieldValue.Set(reflect.ValueOf(convertedValue)) - if provenanceFields != nil { + if provenanceFields != nil && sourceName != "" { sourceInfo := sourceName if found && entry.sourceKey != "" { sourceInfo = entry.sourceKey diff --git a/loader.go b/loader.go index ea39910..3164c81 100644 --- a/loader.go +++ b/loader.go @@ -111,11 +111,12 @@ func (l *Loader[T]) loadInternal(ctx context.Context, store bool) (*T, *Provenan // Get all valid field keys from the struct var cfg T validKeys := collectValidKeys(reflect.TypeOf(cfg), "") + dynamicMapKeyPatterns := collectDynamicMapKeyPatterns(reflect.TypeOf(cfg), "") // Check for unknown keys var unknownKeyErrors []FieldError for key := range mergedData { - if !validKeys[key] { + if !validKeys[key] && !matchesDynamicMapKeyPattern(key, dynamicMapKeyPatterns) { unknownKeyErrors = append(unknownKeyErrors, FieldError{ FieldPath: key, Code: ErrCodeUnknownKey, @@ -257,6 +258,114 @@ func collectValidKeys(t reflect.Type, prefix string) map[string]bool { return validKeys } +func collectDynamicMapKeyPatterns(t reflect.Type, prefix string) []string { + patternSet := make(map[string]struct{}) + + var walk func(reflect.Type, string) + walk = func(currType reflect.Type, currPrefix string) { + if currType.Kind() == reflect.Ptr { + currType = currType.Elem() + } + if currType.Kind() != reflect.Struct { + return + } + + for _, meta := range getStructFieldMeta(currType) { + field := meta.field + tagCfg := meta.tagCfg + keyPath := determineKeyPath(field.Name, tagCfg, currPrefix) + + fieldType := field.Type + unwrappedType := fieldType + if isOptionalType(unwrappedType) { + unwrappedType = unwrappedType.Field(0).Type + } + + if unwrappedType.Kind() == reflect.Map && unwrappedType.Key().Kind() == reflect.String { + for _, pattern := range collectMapValueKeyPatterns(keyPath, unwrappedType.Elem()) { + patternSet[pattern] = struct{}{} + } + } + + if isOptionalType(fieldType) { + innerType := fieldType.Field(0).Type + if innerType.Kind() == reflect.Struct { + walk(innerType, keyPath) + } + continue + } + + if fieldType.Kind() != reflect.Struct || fieldType.PkgPath() == "time" { + continue + } + + nestedPrefix := keyPath + if tagCfg.prefix != "" { + nestedPrefix = tagCfg.prefix + } + walk(fieldType, nestedPrefix) + } + } + + walk(t, prefix) + + patterns := make([]string, 0, len(patternSet)) + for pattern := range patternSet { + patterns = append(patterns, pattern) + } + + return patterns +} + +func collectMapValueKeyPatterns(baseKey string, valueType reflect.Type) []string { + if valueType.Kind() == reflect.Ptr { + valueType = valueType.Elem() + } + + if isOptionalType(valueType) { + valueType = valueType.Field(0).Type + } + + if valueType.Kind() == reflect.Struct && valueType.PkgPath() != "time" { + patterns := make([]string, 0) + for key := range collectValidKeys(valueType, baseKey+".*") { + patterns = append(patterns, key) + } + return patterns + } + + return []string{baseKey + ".*"} +} + +func matchesDynamicMapKeyPattern(key string, patterns []string) bool { + for _, pattern := range patterns { + if matchDotKeyPattern(pattern, key) { + return true + } + } + + return false +} + +func matchDotKeyPattern(pattern string, key string) bool { + patternParts := strings.Split(pattern, ".") + keyParts := strings.Split(key, ".") + if len(patternParts) != len(keyParts) { + return false + } + + for i := range patternParts { + if patternParts[i] == "*" { + continue + } + if patternParts[i] != keyParts[i] { + return false + } + } + + return true +} + // watchLoop is the main goroutine that monitors sources for changes and reloads configuration. // It handles debouncing, thread-safe snapshot emission, and cleanup. func (l *Loader[T]) watchLoop(ctx context.Context, initialCfg *T, snapshotCh chan<- Snapshot[T], errorCh chan<- error) { diff --git a/loader_test.go b/loader_test.go index 8e73a0b..69b0e14 100644 --- a/loader_test.go +++ b/loader_test.go @@ -734,6 +734,127 @@ func TestLoad_NestedStruct(t *testing.T) { } } +func TestLoad_NestedCollections(t *testing.T) { + type ClickHouseConfig struct { + Host string + Port int + } + + type Config struct { + ClickhouseList []ClickHouseConfig + ClickhouseMap map[string]ClickHouseConfig + } + + t.Run("binds slice and map from yaml-shaped data", func(t *testing.T) { + source := &mockSource{ + data: map[string]any{ + "clickhouse_list": []any{ + map[string]any{"host": "ch1", "port": 9000}, + map[string]any{"host": "ch2", "port": 9001}, + }, + "clickhouse_map.primary.host": "ch1", + "clickhouse_map.primary.port": 9000, + "clickhouse_map.replica.host": "ch2", + "clickhouse_map.replica.port": 9001, + "clickhouse_map.analytics.host": "ch3", + "clickhouse_map.analytics.port": 9002, + }, + } + + cfg, err := NewLoader[Config]().WithSource(source).Load(context.Background()) + if err != nil { + t.Fatalf("Load failed: %v", err) + } + + wantList := []ClickHouseConfig{ + {Host: "ch1", Port: 9000}, + {Host: "ch2", Port: 9001}, + } + if !reflect.DeepEqual(cfg.ClickhouseList, wantList) { + t.Errorf("ClickhouseList = %#v, want %#v", cfg.ClickhouseList, wantList) + } + + wantMap := map[string]ClickHouseConfig{ + "primary": {Host: "ch1", Port: 9000}, + "replica": {Host: "ch2", Port: 9001}, + "analytics": {Host: "ch3", Port: 9002}, + } + if !reflect.DeepEqual(cfg.ClickhouseMap, wantMap) { + t.Errorf("ClickhouseMap = %#v, want %#v", cfg.ClickhouseMap, wantMap) + } + }) + + t.Run("strict mode rejects unknown nested key in map value", func(t *testing.T) { + source := &mockSource{ + data: map[string]any{ + "clickhouse_map.primary.host": "ch1", + "clickhouse_map.primary.port": 9000, + "clickhouse_map.primary.unknown": "bad", + }, + } + + cfg, err := NewLoader[Config]().WithSource(source).Load(context.Background()) + if err == nil { + t.Fatal("expected strict-mode error for unknown nested map key") + } + if cfg != nil { + t.Fatal("expected nil cfg when strict-mode validation fails") + } + + valErr, ok := err.(*ValidationError) + if !ok { + t.Fatalf("expected ValidationError, got %T", err) + } + + foundUnknown := false + for _, fieldErr := range valErr.FieldErrors { + if fieldErr.Code == ErrCodeUnknownKey && fieldErr.FieldPath == "clickhouse_map.primary.unknown" { + foundUnknown = true + } + } + if !foundUnknown { + t.Fatalf("expected unknown_key for clickhouse_map.primary.unknown, got %#v", valErr.FieldErrors) + } + }) + + t.Run("invalid nested collection type reports invalid_type", func(t *testing.T) { + source := &mockSource{ + data: map[string]any{ + "clickhouse_list": []any{ + map[string]any{"host": "ch1", "port": "not-an-int"}, + }, + }, + } + + cfg, err := NewLoader[Config]().WithSource(source).Load(context.Background()) + if err == nil { + t.Fatal("expected invalid_type error") + } + if cfg != nil { + t.Fatal("expected nil cfg on invalid_type error") + } + + valErr, ok := err.(*ValidationError) + if !ok { + t.Fatalf("expected ValidationError, got %T", err) + } + if len(valErr.FieldErrors) == 0 { + t.Fatal("expected at least one field error") + } + + fieldErr := valErr.FieldErrors[0] + if fieldErr.Code != ErrCodeInvalidType { + t.Fatalf("expected code %q, got %q", ErrCodeInvalidType, fieldErr.Code) + } + if fieldErr.FieldPath != "ClickhouseList" { + t.Fatalf("expected FieldPath %q, got %q", "ClickhouseList", fieldErr.FieldPath) + } + if !strings.Contains(fieldErr.Message, "slice element 0") { + t.Fatalf("expected index context in error message, got %q", fieldErr.Message) + } + }) +} + // TestLoad_SourceError verifies that source load errors are propagated. func TestLoad_SourceError(t *testing.T) { type Config struct { From 701f164bab7128e54b8ef2278c80edaf3f92b282 Mon Sep 17 00:00:00 2001 From: Jabar Asadi Date: Mon, 23 Feb 2026 16:32:06 +0100 Subject: [PATCH 2/2] trim nested map strict support to minimal scope --- binding.go | 73 +------------------------------- loader.go | 111 +------------------------------------------------ loader_test.go | 46 +++----------------- 3 files changed, 8 insertions(+), 222 deletions(-) diff --git a/binding.go b/binding.go index 14936bd..fa0477f 100644 --- a/binding.go +++ b/binding.go @@ -517,70 +517,6 @@ func parseStringSlice(rawValue any) ([]string, error) { } } -func synthesizeNestedMapEntry(data map[string]mergedEntry, keyPath string) (mergedEntry, bool) { - prefix := keyPath + "." - nested := make(map[string]any) - - var sourceName string - var sourceKey string - found := false - mixedSources := false - - for dataKey, entry := range data { - if !strings.HasPrefix(dataKey, prefix) { - continue - } - - parts := strings.Split(strings.TrimPrefix(dataKey, prefix), ".") - if len(parts) == 0 || parts[0] == "" { - continue - } - - setNestedMapValue(nested, parts, entry.value) - if !found { - sourceName = entry.sourceName - sourceKey = entry.sourceKey - found = true - continue - } - - if sourceName != entry.sourceName || sourceKey != entry.sourceKey { - mixedSources = true - } - } - - if !found { - return mergedEntry{}, false - } - - if mixedSources { - // A single field-level provenance source would be misleading for a synthesized mixed-source map. - sourceName = "" - sourceKey = "" - } - - return mergedEntry{ - value: nested, - sourceName: sourceName, - sourceKey: sourceKey, - }, true -} - -func setNestedMapValue(dst map[string]any, parts []string, value any) { - if len(parts) == 1 { - dst[parts[0]] = value - return - } - - child, ok := dst[parts[0]].(map[string]any) - if !ok { - child = make(map[string]any) - dst[parts[0]] = child - } - - setNestedMapValue(child, parts[1:], value) -} - // mergedEntry represents a configuration value with its source information. type mergedEntry struct { value any @@ -702,13 +638,6 @@ func bindScalarField( var rawValue any var sourceName string - if !found && fieldValue.Kind() == reflect.Map { - if synthesizedEntry, ok := synthesizeNestedMapEntry(data, keyPath); ok { - entry = synthesizedEntry - found = true - } - } - if found { rawValue = entry.value sourceName = entry.sourceName @@ -744,7 +673,7 @@ func bindScalarField( if fieldValue.CanSet() { fieldValue.Set(reflect.ValueOf(convertedValue)) - if provenanceFields != nil && sourceName != "" { + if provenanceFields != nil { sourceInfo := sourceName if found && entry.sourceKey != "" { sourceInfo = entry.sourceKey diff --git a/loader.go b/loader.go index 3164c81..ea39910 100644 --- a/loader.go +++ b/loader.go @@ -111,12 +111,11 @@ func (l *Loader[T]) loadInternal(ctx context.Context, store bool) (*T, *Provenan // Get all valid field keys from the struct var cfg T validKeys := collectValidKeys(reflect.TypeOf(cfg), "") - dynamicMapKeyPatterns := collectDynamicMapKeyPatterns(reflect.TypeOf(cfg), "") // Check for unknown keys var unknownKeyErrors []FieldError for key := range mergedData { - if !validKeys[key] && !matchesDynamicMapKeyPattern(key, dynamicMapKeyPatterns) { + if !validKeys[key] { unknownKeyErrors = append(unknownKeyErrors, FieldError{ FieldPath: key, Code: ErrCodeUnknownKey, @@ -258,114 +257,6 @@ func collectValidKeys(t reflect.Type, prefix string) map[string]bool { return validKeys } -func collectDynamicMapKeyPatterns(t reflect.Type, prefix string) []string { - patternSet := make(map[string]struct{}) - - var walk func(reflect.Type, string) - walk = func(currType reflect.Type, currPrefix string) { - if currType.Kind() == reflect.Ptr { - currType = currType.Elem() - } - if currType.Kind() != reflect.Struct { - return - } - - for _, meta := range getStructFieldMeta(currType) { - field := meta.field - tagCfg := meta.tagCfg - keyPath := determineKeyPath(field.Name, tagCfg, currPrefix) - - fieldType := field.Type - unwrappedType := fieldType - if isOptionalType(unwrappedType) { - unwrappedType = unwrappedType.Field(0).Type - } - - if unwrappedType.Kind() == reflect.Map && unwrappedType.Key().Kind() == reflect.String { - for _, pattern := range collectMapValueKeyPatterns(keyPath, unwrappedType.Elem()) { - patternSet[pattern] = struct{}{} - } - } - - if isOptionalType(fieldType) { - innerType := fieldType.Field(0).Type - if innerType.Kind() == reflect.Struct { - walk(innerType, keyPath) - } - continue - } - - if fieldType.Kind() != reflect.Struct || fieldType.PkgPath() == "time" { - continue - } - - nestedPrefix := keyPath - if tagCfg.prefix != "" { - nestedPrefix = tagCfg.prefix - } - walk(fieldType, nestedPrefix) - } - } - - walk(t, prefix) - - patterns := make([]string, 0, len(patternSet)) - for pattern := range patternSet { - patterns = append(patterns, pattern) - } - - return patterns -} - -func collectMapValueKeyPatterns(baseKey string, valueType reflect.Type) []string { - if valueType.Kind() == reflect.Ptr { - valueType = valueType.Elem() - } - - if isOptionalType(valueType) { - valueType = valueType.Field(0).Type - } - - if valueType.Kind() == reflect.Struct && valueType.PkgPath() != "time" { - patterns := make([]string, 0) - for key := range collectValidKeys(valueType, baseKey+".*") { - patterns = append(patterns, key) - } - return patterns - } - - return []string{baseKey + ".*"} -} - -func matchesDynamicMapKeyPattern(key string, patterns []string) bool { - for _, pattern := range patterns { - if matchDotKeyPattern(pattern, key) { - return true - } - } - - return false -} - -func matchDotKeyPattern(pattern string, key string) bool { - patternParts := strings.Split(pattern, ".") - keyParts := strings.Split(key, ".") - if len(patternParts) != len(keyParts) { - return false - } - - for i := range patternParts { - if patternParts[i] == "*" { - continue - } - if patternParts[i] != keyParts[i] { - return false - } - } - - return true -} - // watchLoop is the main goroutine that monitors sources for changes and reloads configuration. // It handles debouncing, thread-safe snapshot emission, and cleanup. func (l *Loader[T]) watchLoop(ctx context.Context, initialCfg *T, snapshotCh chan<- Snapshot[T], errorCh chan<- error) { diff --git a/loader_test.go b/loader_test.go index 69b0e14..542401f 100644 --- a/loader_test.go +++ b/loader_test.go @@ -745,19 +745,18 @@ func TestLoad_NestedCollections(t *testing.T) { ClickhouseMap map[string]ClickHouseConfig } - t.Run("binds slice and map from yaml-shaped data", func(t *testing.T) { + t.Run("binds slice and direct map values", func(t *testing.T) { source := &mockSource{ data: map[string]any{ "clickhouse_list": []any{ map[string]any{"host": "ch1", "port": 9000}, map[string]any{"host": "ch2", "port": 9001}, }, - "clickhouse_map.primary.host": "ch1", - "clickhouse_map.primary.port": 9000, - "clickhouse_map.replica.host": "ch2", - "clickhouse_map.replica.port": 9001, - "clickhouse_map.analytics.host": "ch3", - "clickhouse_map.analytics.port": 9002, + "clickhouse_map": map[string]any{ + "primary": map[string]any{"host": "ch1", "port": 9000}, + "replica": map[string]any{"host": "ch2", "port": 9001}, + "analytics": map[string]any{"host": "ch3", "port": 9002}, + }, }, } @@ -784,39 +783,6 @@ func TestLoad_NestedCollections(t *testing.T) { } }) - t.Run("strict mode rejects unknown nested key in map value", func(t *testing.T) { - source := &mockSource{ - data: map[string]any{ - "clickhouse_map.primary.host": "ch1", - "clickhouse_map.primary.port": 9000, - "clickhouse_map.primary.unknown": "bad", - }, - } - - cfg, err := NewLoader[Config]().WithSource(source).Load(context.Background()) - if err == nil { - t.Fatal("expected strict-mode error for unknown nested map key") - } - if cfg != nil { - t.Fatal("expected nil cfg when strict-mode validation fails") - } - - valErr, ok := err.(*ValidationError) - if !ok { - t.Fatalf("expected ValidationError, got %T", err) - } - - foundUnknown := false - for _, fieldErr := range valErr.FieldErrors { - if fieldErr.Code == ErrCodeUnknownKey && fieldErr.FieldPath == "clickhouse_map.primary.unknown" { - foundUnknown = true - } - } - if !foundUnknown { - t.Fatalf("expected unknown_key for clickhouse_map.primary.unknown, got %#v", valErr.FieldErrors) - } - }) - t.Run("invalid nested collection type reports invalid_type", func(t *testing.T) { source := &mockSource{ data: map[string]any{