From 9e51113ba4d6a308290ed4f9b9a66f5fb5b79085 Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Mon, 12 Sep 2022 08:54:22 -0400 Subject: [PATCH 1/2] Fix:(issue_1197) Set destination field from altsrc for slice flags --- altsrc/flag.go | 6 ++++++ altsrc/yaml_file_loader.go | 4 +--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/altsrc/flag.go b/altsrc/flag.go index 1132ded35f..6e37e537b4 100644 --- a/altsrc/flag.go +++ b/altsrc/flag.go @@ -109,6 +109,9 @@ func (f *StringSliceFlag) ApplyInputSourceValue(cCtx *cli.Context, isc InputSour continue } underlyingFlag.Value = &sliceValue + if f.Destination != nil { + f.Destination.Set(sliceValue.Serialize()) + } } } return nil @@ -137,6 +140,9 @@ func (f *IntSliceFlag) ApplyInputSourceValue(cCtx *cli.Context, isc InputSourceC continue } underlyingFlag.Value = &sliceValue + if f.Destination != nil { + f.Destination.Set(sliceValue.Serialize()) + } } } return nil diff --git a/altsrc/yaml_file_loader.go b/altsrc/yaml_file_loader.go index 315db1885f..62e8965782 100644 --- a/altsrc/yaml_file_loader.go +++ b/altsrc/yaml_file_loader.go @@ -33,11 +33,9 @@ func NewYamlSourceFromFile(file string) (InputSourceContext, error) { // NewYamlSourceFromFlagFunc creates a new Yaml InputSourceContext from a provided flag name and source context. func NewYamlSourceFromFlagFunc(flagFileName string) func(cCtx *cli.Context) (InputSourceContext, error) { return func(cCtx *cli.Context) (InputSourceContext, error) { - if cCtx.IsSet(flagFileName) { - filePath := cCtx.String(flagFileName) + if filePath := cCtx.String(flagFileName); filePath != "" { return NewYamlSourceFromFile(filePath) } - return defaultInputSource() } } From c8dc60b5e6c4b49c1bbc78c303aa8c0d13e611fb Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Mon, 12 Sep 2022 13:40:08 -0400 Subject: [PATCH 2/2] Add unit tests --- altsrc/flag.go | 12 ++++----- altsrc/flag_test.go | 64 ++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 64 insertions(+), 12 deletions(-) diff --git a/altsrc/flag.go b/altsrc/flag.go index 6e37e537b4..8697b29d0a 100644 --- a/altsrc/flag.go +++ b/altsrc/flag.go @@ -109,9 +109,9 @@ func (f *StringSliceFlag) ApplyInputSourceValue(cCtx *cli.Context, isc InputSour continue } underlyingFlag.Value = &sliceValue - if f.Destination != nil { - f.Destination.Set(sliceValue.Serialize()) - } + } + if f.Destination != nil { + f.Destination.Set(sliceValue.Serialize()) } } return nil @@ -140,9 +140,9 @@ func (f *IntSliceFlag) ApplyInputSourceValue(cCtx *cli.Context, isc InputSourceC continue } underlyingFlag.Value = &sliceValue - if f.Destination != nil { - f.Destination.Set(sliceValue.Serialize()) - } + } + if f.Destination != nil { + f.Destination.Set(sliceValue.Serialize()) } } return nil diff --git a/altsrc/flag_test.go b/altsrc/flag_test.go index b2cbb52557..caf62d87f2 100644 --- a/altsrc/flag_test.go +++ b/altsrc/flag_test.go @@ -100,39 +100,61 @@ func TestGenericApplyInputSourceMethodEnvVarSet(t *testing.T) { } func TestStringSliceApplyInputSourceValue_Alias(t *testing.T) { + dest := cli.NewStringSlice() tis := testApplyInputSource{ - Flag: NewStringSliceFlag(&cli.StringSliceFlag{Name: "test", Aliases: []string{"test_alias"}}), + Flag: NewStringSliceFlag(&cli.StringSliceFlag{Name: "test", Aliases: []string{"test_alias"}, Destination: dest}), FlagName: "test_alias", MapValue: []interface{}{"hello", "world"}, } c := runTest(t, tis) expect(t, c.StringSlice("test_alias"), []string{"hello", "world"}) + expect(t, dest.Value(), []string{"hello", "world"}) + // reset dest + dest = cli.NewStringSlice() + tis = testApplyInputSource{ + Flag: NewStringSliceFlag(&cli.StringSliceFlag{Name: "test", Aliases: []string{"test_alias"}, Destination: dest}), + FlagName: "test_alias", + MapValue: []interface{}{"hello", "world"}, + } c = runRacyTest(t, tis) refute(t, c.StringSlice("test_alias"), []string{"hello", "world"}) + refute(t, dest.Value(), []string{"hello", "world"}) } func TestStringSliceApplyInputSourceValue(t *testing.T) { + dest := cli.NewStringSlice() tis := testApplyInputSource{ - Flag: NewStringSliceFlag(&cli.StringSliceFlag{Name: "test"}), + Flag: NewStringSliceFlag(&cli.StringSliceFlag{Name: "test", Destination: dest}), FlagName: "test", MapValue: []interface{}{"hello", "world"}, } c := runTest(t, tis) expect(t, c.StringSlice("test"), []string{"hello", "world"}) + expect(t, dest.Value(), []string{"hello", "world"}) + // reset dest + dest = cli.NewStringSlice() + tis = testApplyInputSource{ + Flag: NewStringSliceFlag(&cli.StringSliceFlag{Name: "test", Destination: dest}), + FlagName: "test", + MapValue: []interface{}{"hello", "world"}, + } c = runRacyTest(t, tis) refute(t, c.StringSlice("test"), []string{"hello", "world"}) + refute(t, dest.Value(), []string{"hello", "world"}) } func TestStringSliceApplyInputSourceMethodContextSet(t *testing.T) { + dest := cli.NewStringSlice() c := runTest(t, testApplyInputSource{ - Flag: NewStringSliceFlag(&cli.StringSliceFlag{Name: "test"}), + Flag: NewStringSliceFlag(&cli.StringSliceFlag{Name: "test", Destination: dest}), FlagName: "test", MapValue: []interface{}{"hello", "world"}, ContextValueString: "ohno", }) expect(t, c.StringSlice("test"), []string{"ohno"}) + expect(t, dest.Value(), []string{"ohno"}) } func TestStringSliceApplyInputSourceMethodEnvVarSet(t *testing.T) { @@ -151,43 +173,73 @@ func TestStringSliceApplyInputSourceMethodEnvVarSet(t *testing.T) { } func TestIntSliceApplyInputSourceValue_Alias(t *testing.T) { + dest := cli.NewIntSlice() tis := testApplyInputSource{ - Flag: NewIntSliceFlag(&cli.IntSliceFlag{Name: "test", Aliases: []string{"test_alias"}}), + Flag: NewIntSliceFlag(&cli.IntSliceFlag{Name: "test", Aliases: []string{"test_alias"}, Destination: dest}), FlagName: "test_alias", MapValue: []interface{}{1, 2}, } c := runTest(t, tis) expect(t, c.IntSlice("test_alias"), []int{1, 2}) + expect(t, dest.Value(), []int{1, 2}) + dest = cli.NewIntSlice() + tis = testApplyInputSource{ + Flag: NewIntSliceFlag(&cli.IntSliceFlag{Name: "test", Aliases: []string{"test_alias"}, Destination: dest}), + FlagName: "test_alias", + MapValue: []interface{}{1, 2}, + } c = runRacyTest(t, tis) refute(t, c.IntSlice("test_alias"), []int{1, 2}) + refute(t, dest.Value(), []int{1, 2}) } func TestIntSliceApplyInputSourceValue(t *testing.T) { + dest := cli.NewIntSlice() tis := testApplyInputSource{ - Flag: NewIntSliceFlag(&cli.IntSliceFlag{Name: "test"}), + Flag: NewIntSliceFlag(&cli.IntSliceFlag{Name: "test", Destination: dest}), FlagName: "test", MapValue: []interface{}{1, 2}, } c := runTest(t, tis) expect(t, c.IntSlice("test"), []int{1, 2}) + expect(t, dest.Value(), []int{1, 2}) + // reset dest + dest = cli.NewIntSlice() + tis = testApplyInputSource{ + Flag: NewIntSliceFlag(&cli.IntSliceFlag{Name: "test", Destination: dest}), + FlagName: "test", + MapValue: []interface{}{1, 2}, + } c = runRacyTest(t, tis) refute(t, c.IntSlice("test"), []int{1, 2}) + refute(t, dest.Value(), []int{1, 2}) } func TestIntSliceApplyInputSourceMethodContextSet(t *testing.T) { + dest := cli.NewIntSlice() tis := testApplyInputSource{ - Flag: NewIntSliceFlag(&cli.IntSliceFlag{Name: "test"}), + Flag: NewIntSliceFlag(&cli.IntSliceFlag{Name: "test", Destination: dest}), FlagName: "test", MapValue: []interface{}{1, 2}, ContextValueString: "3", } c := runTest(t, tis) expect(t, c.IntSlice("test"), []int{3}) + expect(t, dest.Value(), []int{3}) + // reset dest + dest = cli.NewIntSlice() + tis = testApplyInputSource{ + Flag: NewIntSliceFlag(&cli.IntSliceFlag{Name: "test", Destination: dest}), + FlagName: "test", + MapValue: []interface{}{1, 2}, + ContextValueString: "3", + } c = runRacyTest(t, tis) refute(t, c.IntSlice("test"), []int{3}) + refute(t, dest.Value(), []int{3}) } func TestIntSliceApplyInputSourceMethodEnvVarSet(t *testing.T) {