Prevent duplicate entries in board_manager.additional_urls (#1902)

* uniquify configuration arrays in `config add`

* Add tests for unique board manager URLs

* fix linting

* lint: update function name

* update config set to uniquify slices

Author: henrygab

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 uniquifyStringSlice(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",
@@ -55,6 +97,7 @@ func runAddCommand(cmd *cobra.Command, args []string) {
 
 	v := configuration.Settings.GetStringSlice(key)
 	v = append(v, args[1:]...)
+	v = uniquifyStringSlice(v)
 	configuration.Settings.Set(key, v)
 
 	if err := configuration.Settings.WriteConfig(); err != nil {

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:

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,31 @@ 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