From f0851c97a0855b9b62d845f5bba0331a9ef185fd Mon Sep 17 00:00:00 2001 From: Laurent Demailly Date: Mon, 20 Feb 2023 20:14:53 -0800 Subject: [PATCH] Add a String() to Set and Array so it looks better on UI and symetrical with coma input (#4) * Add a String() to Set so it looks better on UI and symetrical with coma separated input * fix array case too and show in example * fix test (or... made it pass) * actually fix (and add a clearer) test for default value of arrays * add a bunch of set functions * switch for fortio.org/sets --- dyngeneric.go | 33 +++++++++++++----------------- dyngeneric_test.go | 39 ++++++++++++++++++++++++++++++++++++ dynstringset.go | 8 +++++--- dynstringset_test.go | 12 ++++++----- endpoint/endpoint.go | 1 + endpoint/endpoint_test.go | 4 ++-- examples/server_kube/http.go | 6 ++++++ go.mod | 5 +++-- go.sum | 10 +++++---- 9 files changed, 83 insertions(+), 35 deletions(-) diff --git a/dyngeneric.go b/dyngeneric.go index 9e65c94..141a0b2 100644 --- a/dyngeneric.go +++ b/dyngeneric.go @@ -10,6 +10,7 @@ import ( "sync/atomic" "time" + "fortio.org/sets" "golang.org/x/exp/constraints" ) @@ -50,11 +51,9 @@ func (*DynamicBoolValueTag) IsBoolFlag() bool { // ---- Generics section --- -type Set[T comparable] map[T]struct{} - // ValidateDynSetMinElements validates that the given Set has at least x elements. -func ValidateDynSetMinElements[T comparable](count int) func(Set[T]) error { - return func(value Set[T]) error { +func ValidateDynSetMinElements[T comparable](count int) func(sets.Set[T]) error { + return func(value sets.Set[T]) error { if len(value) < count { return fmt.Errorf("value set %+v must have at least %v elements", value, count) } @@ -62,7 +61,7 @@ func ValidateDynSetMinElements[T comparable](count int) func(Set[T]) error { } } -// ValidateDynSliceMinElements validates that the given Set has at least x elements. +// ValidateDynSliceMinElements validates that the given array has at least x elements. func ValidateDynSliceMinElements[T any](count int) func([]T) error { return func(value []T) error { if len(value) < count { @@ -75,7 +74,7 @@ func ValidateDynSliceMinElements[T any](count int) func([]T) error { // DynValueTypes are the types currently supported by Parse[T] and thus by Dyn[T]. // DynJSON is special. type DynValueTypes interface { - bool | time.Duration | float64 | int64 | string | []string | Set[string] + bool | time.Duration | float64 | int64 | string | []string | sets.Set[string] } type DynValue[T any] struct { @@ -119,7 +118,7 @@ func FlagSet[T DynValueTypes](flagSet *flag.FlagSet, name string, dynValue *DynV dynValue.flagSet = flagSet dynValue.flagName = name flagSet.Var(dynValue, name, dynValue.usage) - flagSet.Lookup(name).DefValue = fmt.Sprintf("%v", dynValue.av.Load()) + flagSet.Lookup(name).DefValue = dynValue.String() return dynValue } @@ -194,8 +193,8 @@ func parse[T any](input string) (val T, err error) { *v = input case *[]string: *v = CommaStringToSlice(input) - case *Set[string]: - *v = SetFromSlice(CommaStringToSlice(input)) + case *sets.Set[string]: + *v = sets.FromSlice(CommaStringToSlice(input)) default: // JSON Set() and thus Parse() is handled in dynjson.go err = fmt.Errorf("unexpected type %T", val) @@ -203,15 +202,6 @@ func parse[T any](input string) (val T, err error) { return } -// SetFromSlice constructs a Set from a slice. -func SetFromSlice[T comparable](items []T) Set[T] { - res := map[T]struct{}{} - for _, item := range items { - res[item] = struct{}{} - } - return res -} - // Set updates the value from a string representation in a thread-safe manner. // This operation may return an error if the provided `input` doesn't parse, or the resulting value doesn't pass an // optional validator. @@ -282,7 +272,12 @@ func (d *DynValue[T]) Type() string { // String returns the canonical string representation of the type. func (d *DynValue[T]) String() string { - return fmt.Sprintf("%v", d.Get()) + switch v := any(d.Get()).(type) { + case []string: + return strings.Join(v, ",") + default: + return fmt.Sprintf("%v", d.Get()) + } } // WithValueMutator adds a function that changes the value of a flag as needed. diff --git a/dyngeneric_test.go b/dyngeneric_test.go index 2240417..64535ed 100644 --- a/dyngeneric_test.go +++ b/dyngeneric_test.go @@ -8,6 +8,7 @@ import ( "testing" "fortio.org/assert" + "fortio.org/sets" ) // Additional generic tests, most tests are covered by the old per type tests. @@ -31,3 +32,41 @@ func TestDflag_NonDynamic(t *testing.T) { assert.True(t, static != nil) assert.False(t, IsFlagDynamic(static)) } + +func TestSetToString(t *testing.T) { + s := sets.Set[string]{"z": {}, "a": {}, "c": {}, "b": {}} + f := New(s, "test set") + assert.Equal(t, "a,b,c,z", s.String()) + assert.Equal(t, "a,b,c,z", f.Get().String()) +} + +func TestArrayToString(t *testing.T) { + s := []string{"z", "a", "c", "b"} + f := New(s, "test array") + Flag("testing123", f) + defValue := flag.CommandLine.Lookup("testing123").DefValue + // order preserved unlike for sets.Set where we sort + str := f.String() + assert.Equal(t, "z,a,c,b", str) + assert.Equal(t, "z,a,c,b", defValue) +} + +func TestRemoveCommon(t *testing.T) { + setA := sets.New("a", "b", "c", "d") + setB := sets.New("b", "d", "e", "f", "g") + setAA := setA.Clone() + setBB := setB.Clone() + sets.RemoveCommon(setAA, setBB) + assert.Equal(t, "a,c", setAA.String()) // removed + assert.Equal(t, "e,f,g", setBB.String()) // added + // Swap order to exercise the optimization on length of iteration + // also check clone is not modifying the original etc + setAA = setB.Clone() // putting B in AA on purpose and vice versa + setBB = setA.Clone() + sets.RemoveCommon(setAA, setBB) + assert.Equal(t, "a,c", setBB.String()) + assert.Equal(t, "e,f,g", setAA.String()) + assert.True(t, setBB.Has("c")) + setBB.Remove("c") + assert.False(t, setBB.Has("c")) +} diff --git a/dynstringset.go b/dynstringset.go index a11f35d..50249cc 100644 --- a/dynstringset.go +++ b/dynstringset.go @@ -6,12 +6,14 @@ package dflag import ( "flag" "fmt" + + "fortio.org/sets" ) // DynStringSet creates a `Flag` that represents `map[string]struct{}` which is safe to change dynamically at runtime. // Unlike `pflag.StringSlice`, consecutive sets don't append to the slice, but override it. func DynStringSet(flagSet *flag.FlagSet, name string, value []string, usage string) *DynStringSetValue { - d := Dyn(flagSet, name, SetFromSlice(value), usage) + d := Dyn(flagSet, name, sets.FromSlice(value), usage) return &DynStringSetValue{d} } @@ -19,7 +21,7 @@ func DynStringSet(flagSet *flag.FlagSet, name string, value []string, usage stri // DynStringSetValue implements a dynamic set of strings. type DynStringSetValue struct { - *DynValue[Set[string]] + *DynValue[sets.Set[string]] } // Contains returns whether the specified string is in the flag. @@ -39,6 +41,6 @@ func (d *DynStringSetValue) String() string { return fmt.Sprintf("%v", arr) } -func ValidateDynStringSetMinElements(count int) func(Set[string]) error { +func ValidateDynStringSetMinElements(count int) func(sets.Set[string]) error { return ValidateDynSetMinElements[string](count) } diff --git a/dynstringset_test.go b/dynstringset_test.go index ea59507..047affc 100644 --- a/dynstringset_test.go +++ b/dynstringset_test.go @@ -1,4 +1,5 @@ // Copyright 2015 Michal Witkowski. All Rights Reserved. +// Copyright 2020-2023 Fortio Authors. All Rights Reserved. // See LICENSE for licensing terms. package dflag @@ -9,15 +10,16 @@ import ( "time" "fortio.org/assert" + "fortio.org/sets" ) func TestDynStringSet_SetAndGet(t *testing.T) { set := flag.NewFlagSet("foobar", flag.ContinueOnError) dynFlag := DynStringSet(set, "some_stringslice_1", []string{"foo", "bar"}, "Use it or lose it") - assert.Equal(t, Set[string]{"foo": {}, "bar": {}}, dynFlag.Get(), "value must be default after create") + assert.Equal(t, sets.New("foo", "bar"), dynFlag.Get(), "value must be default after create") err := set.Set("some_stringslice_1", "car,bar") assert.NoError(t, err, "setting value must succeed") - assert.Equal(t, Set[string]{"car": {}, "bar": {}}, dynFlag.Get(), "value must be set after update") + assert.Equal(t, sets.New("car", "bar"), dynFlag.Get(), "value must be set after update") } func TestDynStringSet_Contains(t *testing.T) { @@ -45,9 +47,9 @@ func TestDynStringSet_FiresValidators(t *testing.T) { func TestDynStringSet_FiresNotifier(t *testing.T) { waitCh := make(chan struct{}, 1) - notifier := func(oldVal Set[string], newVal Set[string]) { - assert.EqualValues(t, Set[string]{"foo": {}, "bar": {}}, oldVal, "old value in notify must match previous value") - assert.EqualValues(t, Set[string]{"car": {}, "far": {}}, newVal, "new value in notify must match set value") + notifier := func(oldVal sets.Set[string], newVal sets.Set[string]) { + assert.EqualValues(t, sets.Set[string]{"foo": {}, "bar": {}}, oldVal, "old value in notify must match previous value") + assert.EqualValues(t, sets.Set[string]{"car": {}, "far": {}}, newVal, "new value in notify must match set value") waitCh <- struct{}{} } diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index 1aeafbf..80d8e3f 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -1,4 +1,5 @@ // Copyright 2015 Michal Witkowski. All Rights Reserved. +// Copyright 2020-2023 Fortio Authors. All Rights Reserved. // See LICENSE for licensing terms. package endpoint diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go index a895cb1..caa2764 100644 --- a/endpoint/endpoint_test.go +++ b/endpoint/endpoint_test.go @@ -97,8 +97,8 @@ func (s *endpointTestSuite) TestCorrectlyRepresentsResources() { &flagJSON{ Name: "some_dyn_stringslice", Description: "Some dynamic slice text", - CurrentValue: "[car star]", - DefaultValue: "[foo bar]", + CurrentValue: "car,star", + DefaultValue: "foo,bar", IsChanged: true, IsDynamic: true, }, diff --git a/examples/server_kube/http.go b/examples/server_kube/http.go index acaec51..c140501 100644 --- a/examples/server_kube/http.go +++ b/examples/server_kube/http.go @@ -1,4 +1,5 @@ // Copyright 2016 Michal Witkowski. All Rights Reserved. +// Copyright 2020-2023 Fortio Authors. All Rights Reserved. // See LICENSE for licensing terms. package main @@ -14,6 +15,7 @@ import ( "fortio.org/dflag/dynloglevel" "fortio.org/dflag/endpoint" "fortio.org/log" + "fortio.org/sets" ) var ( @@ -47,11 +49,15 @@ var ( }, }, "An arbitrary JSON struct.") + dynArray = dflag.New([]string{"z", "b", "a"}, "An array of strings (comma separated)") + dynSet = dflag.New(sets.New("z", "b", "a"), "An set of strings (comma separated)") ) func main() { dflag.FlagBool("example_bool3", dynBool3) dflag.Flag("example_str2", dynStr2) + dflag.Flag("example_array", dynArray) + dflag.Flag("example_set", dynSet) dynloglevel.LoggerFlagSetup() flag.Parse() u, err := configmap.Setup(flag.CommandLine, *dirPathWatch) diff --git a/go.mod b/go.mod index 8195536..564fabb 100644 --- a/go.mod +++ b/go.mod @@ -3,10 +3,11 @@ module fortio.org/dflag go 1.19 require ( - fortio.org/assert v1.1.3 + fortio.org/assert v1.1.4 fortio.org/log v1.2.2 + fortio.org/sets v0.3.0 github.com/fsnotify/fsnotify v1.6.0 - golang.org/x/exp v0.0.0-20230210204819-062eb4c674ab + golang.org/x/exp v0.0.0-20230213192124-5e25df0256eb ) require golang.org/x/sys v0.4.0 // indirect diff --git a/go.sum b/go.sum index c6b5055..7c1d027 100644 --- a/go.sum +++ b/go.sum @@ -1,11 +1,13 @@ -fortio.org/assert v1.1.3 h1:zXm8xiNiKvq2xG/YQ3sONAg3287XUuklKIDdjyD9pyg= -fortio.org/assert v1.1.3/go.mod h1:039mG+/iYDPO8Ibx8TrNuJCm2T2SuhwRI3uL9nHTTls= +fortio.org/assert v1.1.4 h1:Za1RaG+OjsTMpQS3J3UCvTF6wc4+IOHCz+jAOU37Y4o= +fortio.org/assert v1.1.4/go.mod h1:039mG+/iYDPO8Ibx8TrNuJCm2T2SuhwRI3uL9nHTTls= fortio.org/log v1.2.2 h1:vs42JjNwiqbMbacittZjJE9+oi72Za6aekML9gKmILg= fortio.org/log v1.2.2/go.mod h1:u/8/2lyczXq52aT5Nw6reD+3cR6m/EbS2jBiIYhgiTU= +fortio.org/sets v0.3.0 h1:StetVwMKYsatjzWuglrlElPaCA054zwiVUhDWTSg6wU= +fortio.org/sets v0.3.0/go.mod h1:xVjulHr0FhlmReSymI+AhDtQ4FgjiazQ3JmuNpYFMs8= github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY= github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbSClcnxKAGw= -golang.org/x/exp v0.0.0-20230210204819-062eb4c674ab h1:628ME69lBm9C6JY2wXhAph/yjN3jezx1z7BIDLUwxjo= -golang.org/x/exp v0.0.0-20230210204819-062eb4c674ab/go.mod h1:CxIveKay+FTh1D0yPZemJVgC/95VzuuOLq5Qi4xnoYc= +golang.org/x/exp v0.0.0-20230213192124-5e25df0256eb h1:PaBZQdo+iSDyHT053FjUCgZQ/9uqVwPOcl7KSWhKn6w= +golang.org/x/exp v0.0.0-20230213192124-5e25df0256eb/go.mod h1:CxIveKay+FTh1D0yPZemJVgC/95VzuuOLq5Qi4xnoYc= golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.4.0 h1:Zr2JFtRQNX3BCZ8YtxRE9hNJYC8J6I1MVbMg6owUp18= golang.org/x/sys v0.4.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=