From 861919ed1dcaa2b03dc3c1eed1168aad94db2bf9 Mon Sep 17 00:00:00 2001 From: Henry Gabryjelski Date: Sun, 2 Oct 2022 15:22:21 -0700 Subject: [PATCH 1/5] uniquify configuration arrays in `config add` --- cli/config/add.go | 48 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/cli/config/add.go b/cli/config/add.go index b4ec7824483..3a3d101abed 100644 --- a/cli/config/add.go +++ b/cli/config/add.go @@ -26,6 +26,48 @@ import ( "github.com/spf13/cobra" ) +// // TODO: When update to go 1.18 or later, convert to generic +// // to allow uniquify() on any slice that supports +// // `comparable` +// // See https://gosamples.dev/generics-remove-duplicates-slice/ +// func uniquify[T comparable](s []T) []T { +// // use a map, which enforces unique keys +// inResult := make(map[T]bool) +// var result []T +// // loop through input slice **in order**, +// // to ensure output retains that order +// // (except that it removes duplicates) +// for i := 0; i < len(s); i++ { +// // attempt to use the element as a key +// if _, ok := inResult[s[i]]; !ok { +// // if key didn't exist in map, +// // add to map and append to result +// inResult[s[i]] = true +// result = append(result, s[i]) +// } +// } +// return result +// } + +func uniquify_string_slice(s []string) []string { + // use a map, which enforces unique keys + inResult := make(map[string]bool) + var result []string + // loop through input slice **in order**, + // to ensure output retains that order + // (except that it removes duplicates) + for i := 0; i < len(s); i++ { + // attempt to use the element as a key + if _, ok := inResult[s[i]]; !ok { + // if key didn't exist in map, + // add to map and append to result + inResult[s[i]] = true + result = append(result, s[i]) + } + } + return result +} + func initAddCommand() *cobra.Command { addCommand := &cobra.Command{ Use: "add", @@ -54,7 +96,13 @@ func runAddCommand(cmd *cobra.Command, args []string) { } v := configuration.Settings.GetStringSlice(key) + // only insert values that do not already exist + // old code appended all except the first arg (which was the key) v = append(v, args[1:]...) + // v now has the original values + the appended values + // but, the appended values might have already existed + // if so, remove them. + v = uniquify_string_slice(v) configuration.Settings.Set(key, v) if err := configuration.Settings.WriteConfig(); err != nil { From 9fb7b17aaf07d7546bcbc5ef3fa192680799fb8c Mon Sep 17 00:00:00 2001 From: Henry Gabryjelski Date: Sun, 2 Oct 2022 15:58:28 -0700 Subject: [PATCH 2/5] Add tests for unique board manager URLs --- test/test_config.py | 62 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/test/test_config.py b/test/test_config.py index ab49ac5caeb..36f4aa36a82 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -277,6 +277,16 @@ def test_add_single_argument(run_command): settings_json = json.loads(result.stdout) assert ["https://example.com"] == settings_json["board_manager"]["additional_urls"] + # Adds the same URL (should not error) + url = "https://example.com" + assert run_command(["config", "add", "board_manager.additional_urls", url]) + + # Verifies a second copy has NOT been added + result = run_command(["config", "dump", "--format", "json"]) + assert result.ok + settings_json = json.loads(result.stdout) + assert ["https://example.com"] == settings_json["board_manager"]["additional_urls"] + def test_add_multiple_arguments(run_command): # Create a config file @@ -303,6 +313,34 @@ def test_add_multiple_arguments(run_command): assert urls[0] in settings_json["board_manager"]["additional_urls"] assert urls[1] in settings_json["board_manager"]["additional_urls"] + # Adds both the same URLs a second time + assert run_command(["config", "add", "board_manager.additional_urls"] + urls) + + # Verifies no change in result array + result = run_command(["config", "dump", "--format", "json"]) + assert result.ok + settings_json = json.loads(result.stdout) + assert 2 == len(settings_json["board_manager"]["additional_urls"]) + assert urls[0] in settings_json["board_manager"]["additional_urls"] + assert urls[1] in settings_json["board_manager"]["additional_urls"] + + # Adds multiple URLs ... the middle one is the only new URL + urls = [ + "https://example.com/package_example_index.json", + "https://example.com/a_third_package_example_index.json", + "https://example.com/yet_another_package_example_index.json", + ] + assert run_command(["config", "add", "board_manager.additional_urls"] + urls) + + # Verifies URL has been saved + result = run_command(["config", "dump", "--format", "json"]) + assert result.ok + settings_json = json.loads(result.stdout) + assert 3 == len(settings_json["board_manager"]["additional_urls"]) + assert urls[0] in settings_json["board_manager"]["additional_urls"] + assert urls[1] in settings_json["board_manager"]["additional_urls"] + assert urls[2] in settings_json["board_manager"]["additional_urls"] + def test_add_on_unsupported_key(run_command): # Create a config file @@ -482,6 +520,30 @@ def test_set_slice_with_multiple_arguments(run_command): assert urls[0] in settings_json["board_manager"]["additional_urls"] assert urls[1] in settings_json["board_manager"]["additional_urls"] + # Sets a third set of 7 URLs (with only 4 unique values) + urls = [ + "https://example.com/first_package_index.json", + "https://example.com/second_package_index.json", + "https://example.com/first_package_index.json", + "https://example.com/fifth_package_index.json", + "https://example.com/second_package_index.json", + "https://example.com/sixth_package_index.json", + "https://example.com/first_package_index.json", + ] + assert run_command(["config", "set", "board_manager.additional_urls"] + urls) + + # Verifies all unique values exist in config + result = run_command(["config", "dump", "--format", "json"]) + assert result.ok + settings_json = json.loads(result.stdout) + assert 4 == len(settings_json["board_manager"]["additional_urls"]) + assert urls[0] in settings_json["board_manager"]["additional_urls"] + assert urls[1] in settings_json["board_manager"]["additional_urls"] + assert urls[2] in settings_json["board_manager"]["additional_urls"] + assert urls[3] in settings_json["board_manager"]["additional_urls"] + assert urls[4] in settings_json["board_manager"]["additional_urls"] + assert urls[5] in settings_json["board_manager"]["additional_urls"] + assert urls[6] in settings_json["board_manager"]["additional_urls"] def test_set_string_with_single_argument(run_command): # Create a config file From 370f637e17aead9f5058607706ad1b1982c84854 Mon Sep 17 00:00:00 2001 From: Henry Gabryjelski Date: Sun, 2 Oct 2022 16:38:16 -0700 Subject: [PATCH 3/5] fix linting --- test/test_config.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_config.py b/test/test_config.py index 36f4aa36a82..fe974775207 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -545,6 +545,7 @@ def test_set_slice_with_multiple_arguments(run_command): assert urls[5] in settings_json["board_manager"]["additional_urls"] assert urls[6] in settings_json["board_manager"]["additional_urls"] + def test_set_string_with_single_argument(run_command): # Create a config file assert run_command(["config", "init", "--dest-dir", "."]) From 24acf276caad7716b7ef9e2dbfef0127266eca2e Mon Sep 17 00:00:00 2001 From: Henry Gabryjelski Date: Sun, 2 Oct 2022 16:40:15 -0700 Subject: [PATCH 4/5] lint: update function name --- cli/config/add.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/config/add.go b/cli/config/add.go index 3a3d101abed..8f0871088d1 100644 --- a/cli/config/add.go +++ b/cli/config/add.go @@ -49,7 +49,7 @@ import ( // return result // } -func uniquify_string_slice(s []string) []string { +func uniquifyStringSlice(s []string) []string { // use a map, which enforces unique keys inResult := make(map[string]bool) var result []string @@ -102,7 +102,7 @@ func runAddCommand(cmd *cobra.Command, args []string) { // v now has the original values + the appended values // but, the appended values might have already existed // if so, remove them. - v = uniquify_string_slice(v) + v = uniquifyStringSlice(v) configuration.Settings.Set(key, v) if err := configuration.Settings.WriteConfig(); err != nil { From dd854665717844fb7fb40c8c55402a409e04bf01 Mon Sep 17 00:00:00 2001 From: Henry Gabryjelski Date: Sun, 2 Oct 2022 16:45:16 -0700 Subject: [PATCH 5/5] update config set to uniquify slices --- cli/config/add.go | 5 ----- cli/config/set.go | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/cli/config/add.go b/cli/config/add.go index 8f0871088d1..ab27aa316cc 100644 --- a/cli/config/add.go +++ b/cli/config/add.go @@ -96,12 +96,7 @@ func runAddCommand(cmd *cobra.Command, args []string) { } v := configuration.Settings.GetStringSlice(key) - // only insert values that do not already exist - // old code appended all except the first arg (which was the key) v = append(v, args[1:]...) - // v now has the original values + the appended values - // but, the appended values might have already existed - // if so, remove them. v = uniquifyStringSlice(v) configuration.Settings.Set(key, v) diff --git a/cli/config/set.go b/cli/config/set.go index b4da0277336..9577915e755 100644 --- a/cli/config/set.go +++ b/cli/config/set.go @@ -59,7 +59,7 @@ func runSetCommand(cmd *cobra.Command, args []string) { var value interface{} switch kind { case reflect.Slice: - value = args[1:] + value = uniquifyStringSlice(args[1:]) case reflect.String: value = args[1] case reflect.Bool: