diff --git a/VERSION b/VERSION index 4e93974..1defe53 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -v2.0.14 \ No newline at end of file +v2.1.0 diff --git a/alias.go b/alias.go index de6a7ac..60b84c4 100644 --- a/alias.go +++ b/alias.go @@ -5,25 +5,77 @@ import ( "strings" ) +// resolveName returns the canonical fig name for a given name or alias. +// Callers must hold tree.mu (read or write) before calling this. +func (tree *figTree) resolveName(name string) string { + name = strings.ToLower(name) + if canonical, exists := tree.aliases[name]; exists { + return canonical + } + return name +} + +func (tree *figTree) Problems() []error { + tree.mu.RLock() + defer tree.mu.RUnlock() + return append([]error(nil), tree.problems...) +} + func (tree *figTree) WithAlias(name, alias string) Plant { tree.mu.Lock() defer tree.mu.Unlock() + name = strings.ToLower(name) alias = strings.ToLower(alias) - if _, exists := tree.aliases[alias]; exists { + + // Guard: alias already registered + if existing, exists := tree.aliases[alias]; exists { + if existing != name { + tree.problems = append(tree.problems, + fmt.Errorf("WithAlias: alias -%s already maps to -%s, cannot remap to -%s", alias, existing, name)) + } + // idempotent: same alias→name pair is a no-op, not an error return tree } - tree.aliases[alias] = name + + // Guard: canonical fig must exist + if _, exists := tree.figs[name]; !exists { + tree.problems = append(tree.problems, + fmt.Errorf("WithAlias: no fig named -%s", name)) + return tree + } + + // Guard: alias must not shadow an existing fig name + if _, exists := tree.figs[alias]; exists { + tree.problems = append(tree.problems, + fmt.Errorf("WithAlias: alias -%s conflicts with existing fig name", alias)) + return tree + } + + // Guard: alias must not shadow an existing flag (covers both figs and + // any flags registered outside of figtree, e.g. via flagSet.Var directly) + if tree.flagSet.Lookup(alias) != nil { + tree.problems = append(tree.problems, + fmt.Errorf("WithAlias: alias -%s conflicts with existing flag", alias)) + return tree + } + + // Guard: underlying value must be retrievable and correctly typed ptr, ok := tree.values.Load(name) if !ok { - fmt.Println("failed to load -" + name + " value") + tree.problems = append(tree.problems, + fmt.Errorf("WithAlias: no value found for -%s", name)) return tree } value, ok := ptr.(*Value) if !ok { - fmt.Println("failed to cast -" + name + " value") + tree.problems = append(tree.problems, + fmt.Errorf("WithAlias: value for -%s is %T, expected *Value", name, ptr)) return tree } + + // All validations passed — register the alias + tree.aliases[alias] = name tree.flagSet.Var(value, alias, "Alias of -"+name) return tree } diff --git a/alias_test.go b/alias_test.go index a172a39..69098d5 100644 --- a/alias_test.go +++ b/alias_test.go @@ -1,12 +1,210 @@ package figtree import ( + "context" + "fmt" "os" + "path/filepath" + "sync" "testing" + "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func TestAlias_DuplicateAlias_SecondIsNoop(t *testing.T) { + os.Args = []string{os.Args[0]} + figs := With(Options{Germinate: true}) + figs = figs.NewString("alpha", "a-val", "usage") + figs = figs.NewString("beta", "b-val", "usage") + figs = figs.WithAlias("alpha", "x") + figs = figs.WithAlias("beta", "x") // second registration — should be ignored + assert.NoError(t, figs.Parse()) + // "x" should still point to "alpha", not "beta" + assert.Equal(t, "a-val", *figs.String("x")) +} + +func TestAlias_ValidatorOnAlias(t *testing.T) { + os.Args = []string{os.Args[0]} + figs := With(Options{Germinate: true}) + figs = figs.NewString("domain", "", "usage") + figs = figs.WithAlias("domain", "d") + figs = figs.WithValidator("d", AssureStringNotEmpty) // register via alias + assert.Error(t, figs.Parse(), "validator registered via alias should fire") +} + +func TestRecall_ChannelCapacityMatchesHarvest(t *testing.T) { + figs := With(Options{Tracking: true, Harvest: 5, Germinate: true}) + figs.NewString("k", "v", "u") + assert.NoError(t, figs.Parse()) + + figs.Curse() + figs.Recall() + + // Send 5 mutations without a receiver — should not block with harvest=5 + done := make(chan struct{}) + go func() { + for i := 0; i < 5; i++ { + figs.StoreString("k", fmt.Sprintf("v%d", i)) + } + close(done) + }() + select { + case <-done: + // pass + case <-time.After(500 * time.Millisecond): + t.Fatal("StoreString blocked: Recall() channel capacity is too small") + } +} + +func TestAssureBoolTrue_WrongType_ReturnsCorrectErrorType(t *testing.T) { + err := AssureBoolTrue(42) // not a bool + require.Error(t, err) + var e ErrInvalidType + require.ErrorAs(t, err, &e) + assert.Equal(t, tBool, e.Wanted, "ErrInvalidType.Wanted should be tBool, not tString") +} + +func TestAssureBoolFalse_WrongType_ReturnsCorrectErrorType(t *testing.T) { + err := AssureBoolFalse("yes") // not a bool + require.Error(t, err) + var e ErrInvalidType + require.ErrorAs(t, err, &e) + assert.Equal(t, tBool, e.Wanted) +} + +func TestAssureListMinLength_ErrorMessage_ReflectsActualCount(t *testing.T) { + err := AssureListMinLength(5)([]string{"a", "b", "c"}) + require.Error(t, err) + assert.NotContains(t, err.Error(), "empty", + "error should report min/actual length, not 'list is empty'") + assert.Contains(t, err.Error(), "3", "error should mention actual length") + assert.Contains(t, err.Error(), "5", "error should mention required minimum") +} + +func TestPersist_MapValue_ContainsSeparator(t *testing.T) { + os.Args = []string{os.Args[0]} + figs := With(Options{Germinate: true}) + figs.NewMap("cfg", map[string]string{}, "usage") + assert.NoError(t, figs.Parse()) + + // Value contains "=" — old SplitN(1) would lose the value part + figs.StoreMap("cfg", map[string]string{"url": "http://example.com?a=1&b=2"}) + + result := *figs.Map("cfg") + assert.Equal(t, "http://example.com?a=1&b=2", result["url"], + "value containing '=' must be preserved intact") +} + +func TestFigTree_SaveTo_JSON_RoundTrip(t *testing.T) { + path := filepath.Join(t.TempDir(), "out.json") + figs := With(Options{Germinate: true}) + figs.NewString("name", "yahuah", "name") + figs.NewInt("age", 33, "age") + assert.NoError(t, figs.SaveTo(path)) + + figs2 := With(Options{Germinate: true}) + figs2.NewString("name", "", "name") + figs2.NewInt("age", 0, "age") + assert.NoError(t, figs2.ReadFrom(path)) + assert.Equal(t, "yahuah", *figs2.String("name")) + assert.Equal(t, 33, *figs2.Int("age")) +} + +func TestWithValidators_MultipleApplied(t *testing.T) { + os.Args = []string{os.Args[0]} + figs := With(Options{Germinate: true}) + figs.NewString("host", "localhost", "usage") + figs = figs.WithValidators("host", + AssureStringNotEmpty, + AssureStringHasPrefix("local"), + AssureStringLengthLessThan(20), + ) + assert.NoError(t, figs.Parse()) + + figs2 := With(Options{Germinate: true}) + figs2.NewString("host", "", "usage") + figs2 = figs2.WithValidators("host", AssureStringNotEmpty) + assert.Error(t, figs2.Parse()) +} + +func TestAlias_StoreThrough(t *testing.T) { + os.Args = []string{os.Args[0]} + figs := With(Options{Germinate: true, Tracking: true, Harvest: 10}) + figs = figs.NewString("verbose", "false", "usage") + figs = figs.WithAlias("verbose", "v") + assert.NoError(t, figs.Parse()) + + figs.StoreString("v", "true") // store via alias + + assert.Equal(t, "true", *figs.String("verbose"), "canonical should reflect alias store") + assert.Equal(t, "true", *figs.String("v"), "alias should reflect alias store") +} + +func TestWithAlias_ConflictsWithExistingFig(t *testing.T) { + os.Args = []string{os.Args[0]} + figs := With(Options{Germinate: true}) + figs = figs.NewString("long", "default", "usage") + figs = figs.NewString("short", "default", "usage") + + // Attempt to register "short" as an alias for "long" — but "short" is + // already a registered fig name, so this should record a problem and + // not panic. + figs = figs.WithAlias("long", "short") + + assert.NoError(t, figs.Parse()) + problems := figs.(*figTree).Problems() + assert.Len(t, problems, 1, "expected one problem recorded for alias conflict") + assert.Contains(t, problems[0].Error(), "conflicts with existing fig name") +} + +func TestConcurrentPollinateReads(t *testing.T) { + os.Args = []string{os.Args[0]} + figs := With(Options{Pollinate: true, Germinate: true, Tracking: false}) + figs.NewString("concurrent_key", "initial", "usage") + assert.NoError(t, figs.Parse()) + + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + defer os.Unsetenv("CONCURRENT_KEY") + + go func() { + vals := []string{"alpha", "beta", "gamma"} + i := 0 + ticker := time.NewTicker(5 * time.Millisecond) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + os.Setenv("CONCURRENT_KEY", vals[i%3]) + i++ + } + } + }() + + var wg sync.WaitGroup + for n := 0; n < 10; n++ { + wg.Add(1) + go func() { + defer wg.Done() + ticker := time.NewTicker(10 * time.Millisecond) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + _ = figs.String("concurrent_key") + } + } + }() + } + wg.Wait() +} + func TestWithAlias(t *testing.T) { const cmdLong, cmdAliasLong, valueLong, usage = "long", "l", "default", "usage" const cmdShort, cmdAliasShort, valueShort = "short", "s", "default" @@ -25,6 +223,15 @@ func TestWithAlias(t *testing.T) { figs = nil }) + t.Run("shorthand_notation", func(t *testing.T) { + os.Args = []string{os.Args[0], "-" + cmdAliasLong, valueLong} + figs := With(Options{Germinate: true, Tracking: false}) + figs.NewString(cmdLong, valueLong, usage) + figs.WithAlias(cmdLong, cmdAliasLong) + assert.NoError(t, figs.Parse()) + assert.Equal(t, valueLong, *figs.String(cmdLong)) + }) + t.Run("multiple_aliases", func(t *testing.T) { os.Args = []string{os.Args[0]} const k, v, u = "name", "yeshua", "the real name of god" @@ -46,7 +253,11 @@ func TestWithAlias(t *testing.T) { }) t.Run("complex_usage", func(t *testing.T) { - os.Args = []string{os.Args[0]} + os.Args = []string{ + os.Args[0], + "-list", "three,four,five", + "-map", "four=4,five=5,six=6", + } figs := With(Options{Germinate: true, Tracking: false}) // long figs = figs.NewString(cmdLong, valueLong, usage) @@ -58,6 +269,16 @@ func TestWithAlias(t *testing.T) { figs = figs.WithAlias(cmdShort, cmdAliasShort) figs = figs.WithValidator(cmdShort, AssureStringNotEmpty) + // list + figs.NewList("myList", []string{"one", "two", "three"}, "usage") + figs.WithValidator("myList", AssureListNotEmpty) + figs.WithAlias("myList", "list") + + // map + figs.NewMap("myMap", map[string]string{"one": "1", "two": "2", "three": "3"}, "usage") + figs.WithValidator("myMap", AssureMapNotEmpty) + figs.WithAlias("myMap", "map") + assert.NoError(t, figs.Parse()) // long @@ -66,6 +287,19 @@ func TestWithAlias(t *testing.T) { // short assert.Equal(t, valueShort, *figs.String(cmdShort)) assert.Equal(t, valueShort, *figs.String(cmdAliasShort)) + // list + assert.NotEqual(t, []string{"one", "two", "three"}, *figs.List("myList")) + assert.ElementsMatch(t, []string{"five", "four", "three"}, *figs.List("myList")) + + // list alias + assert.NotEqual(t, []string{"one", "two", "three"}, *figs.List("list")) + assert.ElementsMatch(t, []string{"five", "four", "three"}, *figs.List("list")) + // map + assert.NotEqual(t, map[string]string{"one": "1", "two": "2", "three": "3"}, *figs.Map("myMap")) + assert.Equal(t, map[string]string{"four": "4", "five": "5", "six": "6"}, *figs.Map("myMap")) + // map alias + assert.NotEqual(t, map[string]string{"one": "1", "two": "2", "three": "3"}, *figs.Map("map")) + assert.Equal(t, map[string]string{"four": "4", "five": "5", "six": "6"}, *figs.Map("map")) figs = nil }) diff --git a/assure.go b/assure.go index ada64f4..f18cd12 100644 --- a/assure.go +++ b/assure.go @@ -11,7 +11,7 @@ import ( var AssureStringHasSuffix = func(suffix string) FigValidatorFunc { return makeStringValidator( func(s string) bool { return strings.HasSuffix(s, suffix) }, - "string must have suffix %q, got %q", + fmt.Sprintf("string must have suffix %q, got %%q", suffix), ) } @@ -19,7 +19,7 @@ var AssureStringHasSuffix = func(suffix string) FigValidatorFunc { var AssureStringHasPrefix = func(prefix string) FigValidatorFunc { return makeStringValidator( func(s string) bool { return strings.HasPrefix(s, prefix) }, - "string must have prefix %q, got %q", + fmt.Sprintf("string must have prefix %q, got %%q", prefix), ) } @@ -57,7 +57,7 @@ var AssureStringNoPrefixes = func(prefixes []string) FigValidatorFunc { } } -// AssureStringHasSuffixes ensures a string ends with a suffix +// AssureStringHasSuffixes ensures a string ends with all suffixes in the list provided // Returns a figValidatorFunc that checks for the substring (case-sensitive). var AssureStringHasSuffixes = func(suffixes []string) FigValidatorFunc { return func(value interface{}) error { @@ -222,7 +222,7 @@ var AssureStringNotEmpty = func(value interface{}) error { var AssureStringContains = func(substring string) FigValidatorFunc { return makeStringValidator( func(s string) bool { return strings.Contains(s, substring) }, - "string must contain %q, got %q", + fmt.Sprintf("string must contain %q, got %%q", substring), ) } @@ -248,7 +248,7 @@ var AssureBoolTrue = func(value interface{}) error { v := figFlesh{value, nil} if v.IsBool() { if !v.ToBool() { - return fmt.Errorf("value must be true, got false") + return fmt.Errorf("value must be true, got %t", v.ToBool()) } return nil } @@ -261,7 +261,7 @@ var AssureBoolFalse = func(value interface{}) error { v := figFlesh{value, nil} if v.IsBool() { if v.ToBool() { - return fmt.Errorf("value must be false, got true") + return fmt.Errorf("bool must be false, got %t", v.ToBool()) } return nil } @@ -364,7 +364,7 @@ var AssureInt64LessThan = func(below int64) FigValidatorFunc { return func(value interface{}) error { v := figFlesh{value, nil} if !v.IsInt64() { - return fmt.Errorf("value must be int64, got %d", value) + return ErrInvalidType{tInt64, value} } i := v.ToInt64() if i > below { @@ -486,7 +486,7 @@ var AssureDurationGreaterThan = func(above time.Duration) FigValidatorFunc { return func(value interface{}) error { v := figFlesh{value, nil} if !v.IsDuration() { - return fmt.Errorf("value must be a duration, got %v", value) + return ErrInvalidType{tDuration, value} } t := v.ToDuration() if t < above { @@ -502,7 +502,7 @@ var AssureDurationLessThan = func(below time.Duration) FigValidatorFunc { return func(value interface{}) error { v := figFlesh{value, nil} if !v.IsDuration() { - return fmt.Errorf("value must be a duration, got %v", value) + return ErrInvalidType{tDuration, value} } t := v.ToDuration() if t > below { @@ -582,7 +582,7 @@ var AssureListMinLength = func(min int) FigValidatorFunc { } l := v.ToList() if len(l) < min { - return fmt.Errorf("list is empty") + return fmt.Errorf("list must have at least %d elements, got %d", min, len(l)) } return nil } @@ -592,7 +592,7 @@ var AssureListMinLength = func(min int) FigValidatorFunc { // Returns a figValidatorFunc that checks for the presence of the value. var AssureListContains = func(inside string) FigValidatorFunc { return func(value interface{}) error { - v := NewFlesh(value) + v := figFlesh{value, nil} if !v.IsList() { return ErrInvalidType{tList, value} } @@ -712,7 +712,7 @@ var AssureMapValueMatches = func(key, match string) FigValidatorFunc { return func(value interface{}) error { v := figFlesh{value, nil} if !v.IsMap() { - return fmt.Errorf("%s is not a map", key) + return ErrInvalidType{tMap, value} } m := v.ToMap() if val, exists := m[key]; exists { diff --git a/callback.go b/callback.go index 9b0b72c..95a4a9c 100644 --- a/callback.go +++ b/callback.go @@ -2,7 +2,6 @@ package figtree import ( "errors" - "strings" ) // WithCallback allows you to assign a slice of CallbackFunc to a figFruit attached to a figTree. @@ -27,7 +26,7 @@ import ( func (tree *figTree) WithCallback(name string, whenCallback CallbackWhen, runThis CallbackFunc) Plant { tree.mu.Lock() defer tree.mu.Unlock() - name = strings.ToLower(name) + name = tree.resolveName(name) fruit, exists := tree.figs[name] if !exists || fruit == nil { return tree diff --git a/errors.go b/errors.go index ad24871..9b42276 100644 --- a/errors.go +++ b/errors.go @@ -8,6 +8,7 @@ import ( func (tree *figTree) ErrorFor(name string) error { tree.mu.RLock() defer tree.mu.RUnlock() + name = tree.resolveName(name) fruit, exists := tree.figs[name] if !exists || fruit == nil { return fmt.Errorf("no tree named %s", name) diff --git a/figtree.go b/figtree.go index 8430103..eea12aa 100644 --- a/figtree.go +++ b/figtree.go @@ -63,13 +63,17 @@ func Grow() Plant { func With(opts Options) Plant { angel := atomic.Bool{} angel.Store(true) + chBuf := 1 + if opts.Tracking && opts.Harvest > 0 { + chBuf = opts.Harvest + } fig := &figTree{ ConfigFilePath: opts.ConfigFile, ignoreEnv: opts.IgnoreEnvironment, filterTests: opts.Germinate, pollinate: opts.Pollinate, tracking: opts.Tracking, - harvest: opts.Harvest, + harvest: chBuf, angel: &angel, problems: make([]error, 0), aliases: make(map[string]string), @@ -77,7 +81,7 @@ func With(opts Options) Plant { values: &sync.Map{}, withered: make(map[string]witheredFig), mu: sync.RWMutex{}, - mutationsCh: make(chan Mutation), + mutationsCh: make(chan Mutation, chBuf), flagSet: flag.NewFlagSet(os.Args[0], flag.ContinueOnError), } fig.flagSet.Usage = fig.Usage diff --git a/flesh.go b/flesh.go index e8e11c8..4ebbe51 100644 --- a/flesh.go +++ b/flesh.go @@ -168,20 +168,26 @@ func (flesh *figFlesh) ToList() []string { } } -func (flesh *figFlesh) ToMap() map[string]string { - checkString := func(ck string) map[string]string { - f := make(map[string]string) - u := strings.Split(ck, MapSeparator) - for _, i := range u { - r := strings.SplitN(i, MapKeySeparator, 1) - if len(r) == 2 { - f[r[0]] = r[1] - } else { - flesh.Error = fmt.Errorf("invalid key: %s", i) - } +func (flesh *figFlesh) getMapString(in string) map[string]string { + // f — the result map being built + // ff — each key=value pair from the split + // uck — the index of MapKeySeparator within ff + // uck == -1 — no separator found, invalid pair + // ff[:uck] — everything before the separator, the key + // ff[uck+len(MapKeySeparator):] — everything after the separator, the value + f := make(map[string]string) + for _, ff := range strings.Split(in, MapSeparator) { + uck := strings.Index(ff, MapKeySeparator) + if uck == -1 { + flesh.Error = fmt.Errorf("invalid key: %s", ff) + continue } - return f + f[ff[:uck]] = ff[uck+len(MapKeySeparator):] } + return f +} + +func (flesh *figFlesh) ToMap() map[string]string { switch ft := flesh.Flesh.(type) { case *figFlesh: return ft.ToMap() @@ -197,9 +203,9 @@ func (flesh *figFlesh) ToMap() map[string]string { case *map[string]string: return *ft case string: - return checkString(ft) + return flesh.getMapString(ft) case *string: - return checkString(*ft) + return flesh.getMapString(*ft) default: return map[string]string{} } @@ -347,6 +353,15 @@ func (flesh *figFlesh) IsList() bool { } } +func (flesh *figFlesh) getStringBool(in string) bool { + for _, e := range strings.Split(in, MapSeparator) { + if strings.Index(e, MapKeySeparator) == -1 { + return false + } + } + return true +} + // IsMap checks a FigFlesh Flesh and returns a bool // // figFlesh can be a string NAME=YAHUAH,AGE=33,SEX=MALE can be expressed as @@ -364,15 +379,6 @@ func (flesh *figFlesh) IsList() bool { // fmt.Printf("attributes is a %T with %d keys and equals %q\n", // check, len(attributes.ToMap()) > 0, attributes) func (flesh *figFlesh) IsMap() bool { - checkString := func(f string) bool { - p := strings.Split(f, MapSeparator) - ok := false - for _, e := range p { - n := strings.SplitN(e, MapKeySeparator, 1) - ok = ok && len(n) == 2 - } - return ok - } switch f := flesh.Flesh.(type) { case *figFlesh: return f.IsMap() @@ -385,9 +391,9 @@ func (flesh *figFlesh) IsMap() bool { case *map[string]string: return f != nil case string: - return checkString(f) + return flesh.getStringBool(f) case *string: - return checkString(*f) + return flesh.getStringBool(*f) default: return false } diff --git a/internals.go b/internals.go index 7bd182a..26500c3 100644 --- a/internals.go +++ b/internals.go @@ -12,7 +12,6 @@ import ( "time" "github.com/go-ini/ini" - "gopkg.in/yaml.v3" ) // CONFIGURABLE INTERNAL FUNCTIONS @@ -45,70 +44,6 @@ func (tree *figTree) loadJSON(data []byte) error { return tree.setValuesFromMap(jsonData) } -// loadYAML parses the DefaultYAMLFile or the value of the EnvironmentKey or ConfigFilePath into yaml.Unmarshal -func (tree *figTree) loadYAML(data []byte) error { - var yamlData map[string]interface{} - if err := yaml.Unmarshal(data, &yamlData); err != nil { - return err - } - tree.mu.Lock() - defer tree.mu.Unlock() - tree.activateFlagSet() - for n, d := range yamlData { - var fruit *figFruit - var exists bool - if fruit, exists = tree.figs[n]; exists && fruit != nil { - value := tree.useValue(tree.from(fruit.name)) - ds, err := toString(d) - if err != nil { - return fmt.Errorf("unable toString value for %s: %w", n, err) - } - err = value.Set(ds) - if err != nil { - return fmt.Errorf("unable to Set value for %s: %w", n, err) - } - tree.values.Store(fruit.name, value) - continue - } - mut := tree.MutagenesisOf(d) - vf, er := tree.from(n) - if er == nil && vf != nil && strings.EqualFold(string(vf.Mutagensis), string(tree.MutagenesisOf(d))) { - err := vf.Assign(d) - if err != nil { - return fmt.Errorf("unable to assign value for %s: %w", n, err) - } - tree.values.Store(n, vf) - mut = vf.Mutagensis - } else { - value := &Value{ - Value: d, - Mutagensis: mut, - } - tree.values.Store(n, value) - } - fruit = &figFruit{ - name: n, - Mutagenesis: mut, - usage: "Unknown, loaded from config file", - Mutations: make([]Mutation, 0), - Validators: make([]FigValidatorFunc, 0), - Callbacks: make([]Callback, 0), - Rules: make([]RuleKind, 0), - } - withered := witheredFig{ - name: n, - Value: Value{ - Mutagensis: mut, - Value: d, - }, - } - tree.figs[n] = fruit - tree.withered[n] = withered - } - - return nil -} - // loadINI parses the DefaultINIFile or the value of the EnvironmentKey or ConfigFilePath into ini.Load() func (tree *figTree) loadINI(data []byte) error { cfg, err := ini.Load(data) @@ -146,9 +81,13 @@ func (tree *figTree) loadINI(data []byte) error { // setValuesFromMap uses the data map to store the configurable figs func (tree *figTree) setValuesFromMap(data map[string]interface{}) error { + tree.mu.Lock() + defer tree.mu.Unlock() for key, value := range data { - if _, exists := tree.figs[key]; exists { - if err := tree.mutateFig(key, value); err != nil { + name := tree.resolveName(key) + _, exists := tree.figs[name] + if exists { + if err := tree.mutateFig(name, value); err != nil { return fmt.Errorf("error setting key %s: %w", key, err) } } @@ -269,7 +208,7 @@ func (tree *figTree) checkAndSetFromEnv(name string) { // mutateFig replaces the value interface{} and sends a Mutation into Mutations func (tree *figTree) mutateFig(name string, value interface{}) error { - name = strings.ToLower(name) + name = tree.resolveName(name) def, ok := tree.figs[name] if !ok || def == nil { return fmt.Errorf("no such fig: %s", name) diff --git a/internals_test.go b/internals_test.go index f9e40ed..a20c339 100644 --- a/internals_test.go +++ b/internals_test.go @@ -34,7 +34,7 @@ func TestTree_checkAndSetFromEnv(t *testing.T) { values: &sync.Map{}, withered: make(map[string]witheredFig), mu: sync.RWMutex{}, - mutationsCh: make(chan Mutation), + mutationsCh: make(chan Mutation, 10), flagSet: flag.NewFlagSet(os.Args[0], flag.ContinueOnError), } figs.flagSet.Usage = figs.Usage diff --git a/list_flag.go b/list_flag.go index 8d823c5..f56ab0b 100644 --- a/list_flag.go +++ b/list_flag.go @@ -12,6 +12,7 @@ type ListFlag struct { func (tree *figTree) ListValues(name string) []string { tree.mu.Lock() defer tree.mu.Unlock() + name = tree.resolveName(name) if _, exists := tree.figs[name]; !exists { return []string{} } diff --git a/loading.go b/loading.go index d536d3f..fd90518 100644 --- a/loading.go +++ b/loading.go @@ -10,6 +10,7 @@ import ( check "github.com/andreimerlescu/checkfs" "github.com/andreimerlescu/checkfs/file" + "gopkg.in/yaml.v3" ) // Reload will readEnv on each flag in the configurable package @@ -155,6 +156,8 @@ func (tree *figTree) loadFlagSet() (e error) { } */ }() + tree.mu.Lock() + defer tree.mu.Unlock() tree.flagSet.VisitAll(func(f *flag.Flag) { flagName := f.Name for alias, name := range tree.aliases { @@ -233,3 +236,89 @@ func (tree *figTree) loadFlagSet() (e error) { }) return nil } + +// loadYAML parses the DefaultYAMLFile or the value of the EnvironmentKey or ConfigFilePath into yaml.Unmarshal +func (tree *figTree) loadYAML(data []byte) error { + var yamlData map[string]interface{} + if err := yaml.Unmarshal(data, &yamlData); err != nil { + return err + } + tree.mu.Lock() + defer tree.mu.Unlock() + tree.activateFlagSet() + for n, d := range yamlData { + var fruit *figFruit + var exists bool + if fruit, exists = tree.figs[n]; exists && fruit != nil { + value := tree.useValue(tree.from(fruit.name)) + var ds string + var err error + if fruit.Mutagenesis == tMap { + m, merr := toStringMap(d) + if merr != nil { + return fmt.Errorf("unable toStringMap value for %s: %w", n, merr) + } + err = value.Assign(m) + if err != nil { + return fmt.Errorf("unable to Assign map value for %s: %w", n, err) + } + } else if fruit.Mutagenesis == tList { + l, lerr := toStringSlice(d) + if lerr != nil { + return fmt.Errorf("unable toStringSlice value for %s: %w", n, lerr) + } + err = value.Assign(l) + if err != nil { + return fmt.Errorf("unable to Assign list value for %s: %w", n, err) + } + } else { + ds, err = toString(d) + if err != nil { + return fmt.Errorf("unable toString value for %s: %w", n, err) + } + err = value.Set(ds) + if err != nil { + return fmt.Errorf("unable to Set value for %s: %w", n, err) + } + } + tree.values.Store(fruit.name, value) + continue + } + mut := tree.MutagenesisOf(d) + vf, er := tree.from(n) + if er == nil && vf != nil && strings.EqualFold(string(vf.Mutagensis), string(tree.MutagenesisOf(d))) { + err := vf.Assign(d) + if err != nil { + return fmt.Errorf("unable to assign value for %s: %w", n, err) + } + tree.values.Store(n, vf) + mut = vf.Mutagensis + } else { + value := &Value{ + Value: d, + Mutagensis: mut, + } + tree.values.Store(n, value) + } + fruit = &figFruit{ + name: n, + Mutagenesis: mut, + usage: "Unknown, loaded from config file", + Mutations: make([]Mutation, 0), + Validators: make([]FigValidatorFunc, 0), + Callbacks: make([]Callback, 0), + Rules: make([]RuleKind, 0), + } + withered := witheredFig{ + name: n, + Value: Value{ + Mutagensis: mut, + Value: d, + }, + } + tree.figs[n] = fruit + tree.withered[n] = withered + } + + return nil +} diff --git a/map_flag.go b/map_flag.go index 4f22c40..650d708 100644 --- a/map_flag.go +++ b/map_flag.go @@ -19,7 +19,7 @@ func (tree *figTree) MapKeys(name string) []string { defer func() { name = originalName // return value to original }() - name = strings.ToLower(name) + name = tree.resolveName(name) fruit, exists := tree.figs[name] if !exists { return []string{} diff --git a/miracles.go b/miracles.go index 3ee71a6..6d833ce 100644 --- a/miracles.go +++ b/miracles.go @@ -21,6 +21,9 @@ func (tree *figTree) Curse() { // FigFlesh returns a Flesh interface to the Value on the figTree func (tree *figTree) FigFlesh(name string) Flesh { + tree.mu.RLock() + defer tree.mu.RUnlock() + name = tree.resolveName(name) value := tree.useValue(tree.from(name)) return value.Flesh() } diff --git a/mutagenesis.go b/mutagenesis.go index 55f1793..131d577 100644 --- a/mutagenesis.go +++ b/mutagenesis.go @@ -30,6 +30,9 @@ func (m Mutagenesis) Kind() string { // MutagenesisOfFig returns the Mutagensis of the name func (tree *figTree) MutagenesisOfFig(name string) Mutagenesis { + tree.mu.RLock() + defer tree.mu.RUnlock() + name = tree.resolveName(name) fruit, ok := tree.figs[name] if !ok { return "" diff --git a/mutations.go b/mutations.go index 2439466..dae573d 100644 --- a/mutations.go +++ b/mutations.go @@ -34,10 +34,7 @@ func (tree *figTree) String(name string) *string { defer func() { name = originalName // restore after we're done }() - name = strings.ToLower(name) - if _, exists := tree.aliases[name]; exists { - name = tree.aliases[name] - } + name = tree.resolveName(name) fruit, ok := tree.figs[name] if !ok || fruit == nil { return nil @@ -95,10 +92,7 @@ func (tree *figTree) Bool(name string) *bool { defer func() { name = originalName // restore after we're done }() - name = strings.ToLower(name) - if _, exists := tree.aliases[name]; exists { - name = tree.aliases[name] - } + name = tree.resolveName(name) fruit, ok := tree.figs[name] if !ok || fruit == nil { return nil @@ -142,6 +136,7 @@ func (tree *figTree) Bool(name string) *bool { tree.figs[name] = fruit return &zeroBool } + // return the result return &s } @@ -153,10 +148,7 @@ func (tree *figTree) Int(name string) *int { defer func() { name = originalName // restore after we're done }() - name = strings.ToLower(name) - if _, exists := tree.aliases[name]; exists { - name = tree.aliases[name] - } + name = tree.resolveName(name) fruit, ok := tree.figs[name] if !ok || fruit == nil { return nil @@ -216,10 +208,7 @@ func (tree *figTree) Int64(name string) *int64 { defer func() { name = originalName // restore after we're done }() - name = strings.ToLower(name) - if _, exists := tree.aliases[name]; exists { - name = tree.aliases[name] - } + name = tree.resolveName(name) fruit, ok := tree.figs[name] if !ok || fruit == nil { return nil @@ -275,10 +264,7 @@ func (tree *figTree) Float64(name string) *float64 { defer func() { name = originalName // restore after we're done }() - name = strings.ToLower(name) - if _, exists := tree.aliases[name]; exists { - name = tree.aliases[name] - } + name = tree.resolveName(name) fruit, ok := tree.figs[name] if !ok || fruit == nil { return nil @@ -334,10 +320,7 @@ func (tree *figTree) Duration(name string) *time.Duration { defer func() { name = originalName // restore after we're done }() - name = strings.ToLower(name) - if _, exists := tree.aliases[name]; exists { - name = tree.aliases[name] - } + name = tree.resolveName(name) fruit, ok := tree.figs[name] if !ok || fruit == nil { return nil @@ -404,10 +387,7 @@ func (tree *figTree) UnitDuration(name string) *time.Duration { defer func() { name = originalName // restore after we're done }() - name = strings.ToLower(name) - if _, exists := tree.aliases[name]; exists { - name = tree.aliases[name] - } + name = tree.resolveName(name) fruit, ok := tree.figs[name] if !ok || fruit == nil { return nil @@ -474,10 +454,7 @@ func (tree *figTree) List(name string) *[]string { defer func() { name = originalName // restore after we're done }() - name = strings.ToLower(name) - if _, exists := tree.aliases[name]; exists { - name = tree.aliases[name] - } + name = tree.resolveName(name) fruit, ok := tree.figs[name] if !ok || fruit == nil { return nil @@ -496,10 +473,6 @@ func (tree *figTree) List(name string) *[]string { } var v []string switch f := value.Value.(type) { - case string: - v = []string{f} - case *string: - v = []string{*f} case ListFlag: v = make([]string, len(f.values)) copy(v, f.values) @@ -512,6 +485,14 @@ func (tree *figTree) List(name string) *[]string { case []string: v = make([]string, len(f)) copy(v, f) + case string: + fv := strings.Split(f, ListSeparator) + v = make([]string, len(fv)) + copy(v, fv) + case *string: + fv := strings.Split(*f, ListSeparator) + v = make([]string, len(fv)) + copy(v, fv) default: return nil } @@ -532,10 +513,6 @@ func (tree *figTree) List(name string) *[]string { return &zeroList } switch f := value.Value.(type) { - case string: - v = []string{f} - case *string: - v = []string{*f} case ListFlag: v = make([]string, len(f.values)) copy(v, f.values) @@ -548,6 +525,14 @@ func (tree *figTree) List(name string) *[]string { case []string: v = make([]string, len(f)) copy(v, f) + case string: + fv := strings.Split(f, ListSeparator) + v = make([]string, len(fv)) + copy(v, fv) + case *string: + fv := strings.Split(*f, ListSeparator) + v = make([]string, len(fv)) + copy(v, fv) default: panic("unreachable") } @@ -572,10 +557,7 @@ func (tree *figTree) Map(name string) *map[string]string { defer func() { name = originalName // restore after we're done }() - name = strings.ToLower(name) - if _, exists := tree.aliases[name]; exists { - name = tree.aliases[name] - } + name = tree.resolveName(name) fruit, ok := tree.figs[name] if !ok || fruit == nil { return nil @@ -639,7 +621,7 @@ func (tree *figTree) Map(name string) *map[string]string { if !tree.HasRule(RuleNoEnv) && !fruit.HasRule(RuleNoEnv) && !tree.ignoreEnv && tree.pollinate { e := os.Getenv(name) if len(e) > 0 { - i := strings.Split(e, ",") + i := strings.Split(e, MapSeparator) if len(i) == 0 { v = map[string]string{} } else { diff --git a/mutations_store.go b/mutations_store.go index a2ac257..b06768d 100644 --- a/mutations_store.go +++ b/mutations_store.go @@ -3,6 +3,7 @@ package figtree import ( "errors" "fmt" + "slices" "strings" "time" ) @@ -10,6 +11,7 @@ import ( func (tree *figTree) Store(mut Mutagenesis, name string, value interface{}) Plant { tree.mu.Lock() defer tree.mu.Unlock() + name = tree.resolveName(name) fruit, ok := tree.figs[name] if !ok { return tree @@ -57,6 +59,10 @@ func (tree *figTree) Store(mut Mutagenesis, name string, value interface{}) Plan } tree.figs[name] = fruit if tree.tracking && !tree.angel.Load() { + // Store holds tree.mu while sending on mutationsCh. If the channel buffer + // is full, this send will block, stalling other tree operations. Ensure the + // channel capacity (Harvest) is large enough or consume mutations promptly. + tree.mu.Unlock() // fixes classic "lock while sending to a channel whose consumer needs the lock" tree.mutationsCh <- Mutation{ Property: name, Mutagenesis: strings.ToLower(string(mut)), @@ -66,6 +72,7 @@ func (tree *figTree) Store(mut Mutagenesis, name string, value interface{}) Plan When: time.Now(), Error: err, } + tree.mu.Lock() // allows for the defer method to capture the remainder of the functionality of Store() } return tree } @@ -145,7 +152,7 @@ func (tree *figTree) persist(fruit *figFruit, mut Mutagenesis, name string, valu case string: m := map[string]string{} for _, p := range strings.Split(f, MapSeparator) { - z := strings.SplitN(p, MapKeySeparator, 1) + z := strings.SplitN(p, MapKeySeparator, 2) if len(z) == 2 { m[z[0]] = z[1] } @@ -154,7 +161,7 @@ func (tree *figTree) persist(fruit *figFruit, mut Mutagenesis, name string, valu case *string: m := map[string]string{} for _, p := range strings.Split(*f, MapSeparator) { - z := strings.SplitN(p, MapKeySeparator, 1) + z := strings.SplitN(p, MapKeySeparator, 2) if len(z) == 2 { m[z[0]] = z[1] } @@ -178,7 +185,7 @@ func (tree *figTree) persist(fruit *figFruit, mut Mutagenesis, name string, valu case string: m := map[string]string{} for _, p := range strings.Split(f, MapSeparator) { - z := strings.SplitN(p, MapKeySeparator, 1) + z := strings.SplitN(p, MapKeySeparator, 2) if len(z) == 2 { m[z[0]] = z[1] } @@ -187,7 +194,7 @@ func (tree *figTree) persist(fruit *figFruit, mut Mutagenesis, name string, valu case *string: m := map[string]string{} for _, p := range strings.Split(*f, MapSeparator) { - z := strings.SplitN(p, MapKeySeparator, 1) + z := strings.SplitN(p, MapKeySeparator, 2) if len(z) == 2 { m[z[0]] = z[1] } @@ -215,7 +222,18 @@ func (tree *figTree) persist(fruit *figFruit, mut Mutagenesis, name string, valu } tree.values.Store(name, value) tree.figs[name] = fruit - return old != current, old, current + oldMap, _ := toStringMap(flesh) + currentMap, _ := toStringMap(value) + equal := len(oldMap) == len(currentMap) + if equal { + for k, v := range oldMap { + if cv, exists := currentMap[k]; !exists || cv != v { + equal = false + break + } + } + } + return !equal, old, current case tList: var old *[]string var err error @@ -277,7 +295,16 @@ func (tree *figTree) persist(fruit *figFruit, mut Mutagenesis, name string, valu } tree.values.Store(name, value) tree.figs[name] = fruit - return old != current, old, current + changed := false + switch { + case old == nil && current == nil: + changed = false + case old == nil || current == nil: + changed = true + default: + changed = !slices.Equal(*old, *current) + } + return changed, old, current case tUnitDuration: var old time.Duration var err error diff --git a/mutations_store_test.go b/mutations_store_test.go index 90060da..ab1376d 100644 --- a/mutations_store_test.go +++ b/mutations_store_test.go @@ -1,12 +1,75 @@ package figtree import ( + "os" "testing" "time" "github.com/stretchr/testify/assert" ) +func TestPersist_MapFromString(t *testing.T) { + os.Args = []string{os.Args[0]} + figs := With(Options{Germinate: true, Tracking: false}) + figs.NewMap("attrs", map[string]string{"key1": "value1"}, "usage") + assert.NoError(t, figs.Parse()) + + // Manually store the map value as a raw string to force the string path in persist + tree := figs.(*figTree) + tree.mu.Lock() + v := &Value{ + Value: "key2=value2,key3=value3", + Mutagensis: tMap, + } + tree.values.Store("attrs", v) + tree.mu.Unlock() + + // Now Store through the normal path — persist will read the raw string flesh + // and attempt to parse it via SplitN. With limit 1 the map will be empty. + figs.StoreMap("attrs", map[string]string{"key4": "value4"}) + + result := *figs.Map("attrs") + assert.Contains(t, result, "key4", "key4 should be present after StoreMap") + // This will fail with the SplitN bug because the old value parsed from string + // produces an empty map, making the changed check unreliable + assert.Equal(t, map[string]string{"key4": "value4"}, result) +} + +func TestPersist_MapFromStringMutation(t *testing.T) { + os.Args = []string{os.Args[0]} + figs := With(Options{Germinate: true, Tracking: true}) + figs.NewMap("attrs", map[string]string{"key1": "value1"}, "usage") + assert.NoError(t, figs.Parse()) + + tree := figs.(*figTree) + tree.mu.Lock() + tree.values.Store("attrs", &Value{ + Value: "key2=value2,key3=value3", + Mutagensis: tMap, + }) + tree.mu.Unlock() + + mutationFired := make(chan Mutation, 1) + go func() { + select { + case m, ok := <-figs.Mutations(): + if ok { + mutationFired <- m + } + case <-time.After(500 * time.Millisecond): + close(mutationFired) + } + }() + + figs.StoreMap("attrs", map[string]string{"key2": "value2", "key3": "value3"}) + + mutation, fired := <-mutationFired + if fired { + t.Logf("mutation fired: old=%v new=%v", mutation.Old, mutation.New) + assert.Fail(t, "mutation should not have fired — value did not change") + } +} + func TestTree_StoreString(t *testing.T) { const k, u = "test-store-string", "usage" diff --git a/parsing.go b/parsing.go index 0fb8e82..4c096b8 100644 --- a/parsing.go +++ b/parsing.go @@ -21,13 +21,9 @@ func (tree *figTree) useValue(value *Value, err error) *Value { return value } +// from callers must not call this while holding a read lock and trying to acquire a write lock func (tree *figTree) from(name string) (*Value, error) { - flagName := strings.ToLower(name) - for alias, realname := range tree.aliases { - if strings.EqualFold(alias, name) { - flagName = realname - } - } + flagName := tree.resolveName(name) valueAny, ok := tree.values.Load(flagName) if !ok { return nil, errors.New("no value for " + flagName) diff --git a/rules.go b/rules.go index 62c4d39..7e4a0ee 100644 --- a/rules.go +++ b/rules.go @@ -1,7 +1,5 @@ package figtree -import "strings" - type RuleKind int const ( @@ -49,7 +47,7 @@ func (tree *figTree) WithTreeRule(rule RuleKind) Plant { func (tree *figTree) WithRule(name string, rule RuleKind) Plant { tree.mu.Lock() defer tree.mu.Unlock() - name = strings.ToLower(name) + name = tree.resolveName(name) fruit, exists := tree.figs[name] if !exists || fruit == nil { return tree diff --git a/savior.go b/savior.go index 228ad53..3fbf1d4 100644 --- a/savior.go +++ b/savior.go @@ -32,7 +32,18 @@ func (tree *figTree) SaveTo(path string) error { if !ok { return errors.Join(fig.Error, fmt.Errorf("failed to cast %s as *Value ; got %T", fig.name, valueAny)) } - properties[name] = _value.Value + switch v := _value.Value.(type) { + case MapFlag: + properties[name] = v.values + case *MapFlag: + properties[name] = v.values + case ListFlag: + properties[name] = v.values + case *ListFlag: + properties[name] = v.values + default: + properties[name] = _value.Value + } } formatValue := func(val interface{}) string { return fmt.Sprintf("%v", val) diff --git a/savior_test.go b/savior_test.go index 56f3164..896ddda 100644 --- a/savior_test.go +++ b/savior_test.go @@ -34,3 +34,37 @@ func TestFigTree_SaveTo(t *testing.T) { assert.Equal(t, t.Name(), *name) assert.NoError(t, os.RemoveAll(testFile)) } + +func TestFigTree_SaveTo_MapRoundTrip(t *testing.T) { + path := filepath.Join(t.TempDir(), "out.yaml") + + figs := With(Options{Germinate: true}) + figs.NewMap("cfg", map[string]string{"key": "value", "foo": "bar"}, "usage") + // intentionally do NOT call StoreMap — use the raw MapFlag state from NewMap + assert.NoError(t, figs.SaveTo(path)) + + figs2 := With(Options{Germinate: true}) + figs2.NewMap("cfg", map[string]string{}, "usage") + assert.NoError(t, figs2.ReadFrom(path)) + + result := *figs2.Map("cfg") + assert.Equal(t, map[string]string{"key": "value", "foo": "bar"}, result, + "MapFlag should be unwrapped before serialization — got %v", result) +} + +func TestFigTree_SaveTo_ListRoundTrip(t *testing.T) { + path := filepath.Join(t.TempDir(), "out.yaml") + + figs := With(Options{Germinate: true}) + figs.NewList("items", []string{"one", "two", "three"}, "usage") + // intentionally do NOT call StoreList — use the raw ListFlag state from NewList + assert.NoError(t, figs.SaveTo(path)) + + figs2 := With(Options{Germinate: true}) + figs2.NewList("items", []string{}, "usage") + assert.NoError(t, figs2.ReadFrom(path)) + + result := *figs2.List("items") + assert.Equal(t, []string{"one", "three", "two"}, result, + "ListFlag should be unwrapped before serialization — got %v", result) +} diff --git a/test.config.ini b/test.config.ini index 2ff70b9..85c8d26 100644 --- a/test.config.ini +++ b/test.config.ini @@ -1,3 +1,3 @@ -name: "yahuah" -age: 33 -sex: male \ No newline at end of file +name=yahuah +age=33 +sex=male \ No newline at end of file diff --git a/types.go b/types.go index bc1d9e6..c682d27 100644 --- a/types.go +++ b/types.go @@ -18,6 +18,8 @@ type Withables interface { WithTreeRule(rule RuleKind) Plant // WithValidator binds a figValidatorFunc to a figFruit that returns Plant WithValidator(name string, validator func(interface{}) error) Plant + // WithValidators binds a figValidatorFunc to a figFruit that returns Plant + WithValidators(name string, validators ...func(interface{}) error) Plant } type Savable interface { @@ -56,6 +58,8 @@ type Loadable interface { } type Divine interface { + // Problems exposes non-fatal errors in the figtree like duplicate registrations that get ignored + Problems() []error // Recall allows you to unlock the figTree from changes and resume tracking Recall() // Curse allows you to lock the figTree from changes and stop tracking diff --git a/validators.go b/validators.go index dfbdaf5..affc625 100644 --- a/validators.go +++ b/validators.go @@ -3,7 +3,6 @@ package figtree import ( "fmt" "log" - "strings" "time" ) @@ -19,7 +18,7 @@ import ( func (tree *figTree) WithValidator(name string, validator func(interface{}) error) Plant { tree.mu.Lock() defer tree.mu.Unlock() - name = strings.ToLower(name) + name = tree.resolveName(name) if fig, ok := tree.figs[name]; ok { if fig.HasRule(RuleNoValidations) { return tree