Make RenameYamlKey return a bool

pkg/config/app_config.go

@@ -267,7 +267,7 @@ func computeMigratedConfig(path string, content []byte) ([]byte, bool, error) {
 	}
 
 	for _, pathToReplace := range pathsToReplace {
-		err := yaml_utils.RenameYamlKey(&rootNode, pathToReplace.oldPath, pathToReplace.newName)
+		err, _ := yaml_utils.RenameYamlKey(&rootNode, pathToReplace.oldPath, pathToReplace.newName)
 		if err != nil {
 			return nil, false, fmt.Errorf("Couldn't migrate config file at `%s` for key %s: %s", path, strings.Join(pathToReplace.oldPath, "."), err)
 		}

pkg/utils/yaml_utils/yaml_utils.go

@@ -65,10 +65,10 @@ func transformNode(node *yaml.Node, path []string, transform func(node *yaml.Nod
 
 // Takes the root node of a yaml document, a path to a key, and a new name for the key.
 // Will rename the key to the new name if it exists, and do nothing otherwise.
-func RenameYamlKey(rootNode *yaml.Node, path []string, newKey string) error {
+func RenameYamlKey(rootNode *yaml.Node, path []string, newKey string) (error, bool) {
 	// Empty document: nothing to do.
 	if len(rootNode.Content) == 0 {
-		return nil
+		return nil, false
 	}
 
 	body := rootNode.Content[0]
@@ -77,25 +77,25 @@ func RenameYamlKey(rootNode *yaml.Node, path []string, newKey string) error {
 }
 
 // Recursive function to rename the YAML key.
-func renameYamlKey(node *yaml.Node, path []string, newKey string) error {
+func renameYamlKey(node *yaml.Node, path []string, newKey string) (error, bool) {
 	if node.Kind != yaml.MappingNode {
-		return errors.New("yaml node in path is not a dictionary")
+		return errors.New("yaml node in path is not a dictionary"), false
 	}
 
 	keyNode, valueNode := LookupKey(node, path[0])
 	if keyNode == nil {
-		return nil
+		return nil, false
 	}
 
 	// end of path reached: rename key
 	if len(path) == 1 {
 		// Check that new key doesn't exist yet
 		if newKeyNode, _ := LookupKey(node, newKey); newKeyNode != nil {
-			return fmt.Errorf("new key `%s' already exists", newKey)
+			return fmt.Errorf("new key `%s' already exists", newKey), false
 		}
 
 		keyNode.Value = newKey
-		return nil
+		return nil, true
 	}
 
 	return renameYamlKey(valueNode, path[1:], newKey)

pkg/utils/yaml_utils/yaml_utils_test.go

@@ -10,77 +10,85 @@ import (
 
 func TestRenameYamlKey(t *testing.T) {
 	tests := []struct {
-		name        string
+		name              string
-		in          string
+		in                string
-		path        []string
+		path              []string
-		newKey      string
+		newKey            string
-		expectedOut string
+		expectedOut       string
-		expectedErr string
+		expectedDidRename bool
+		expectedErr       string
 	}{
 		{
-			name:        "rename key",
+			name:              "rename key",
-			in:          "foo: 5\n",
+			in:                "foo: 5\n",
-			path:        []string{"foo"},
+			path:              []string{"foo"},
-			newKey:      "bar",
+			newKey:            "bar",
-			expectedOut: "bar: 5\n",
+			expectedOut:       "bar: 5\n",
-			expectedErr: "",
+			expectedDidRename: true,
+			expectedErr:       "",
 		},
 		{
-			name:        "rename key, nested",
+			name:              "rename key, nested",
-			in:          "foo:\n  bar: 5\n",
+			in:                "foo:\n  bar: 5\n",
-			path:        []string{"foo", "bar"},
+			path:              []string{"foo", "bar"},
-			newKey:      "baz",
+			newKey:            "baz",
-			expectedOut: "foo:\n  baz: 5\n",
+			expectedOut:       "foo:\n  baz: 5\n",
-			expectedErr: "",
+			expectedDidRename: true,
+			expectedErr:       "",
 		},
 		{
-			name:        "rename non-scalar key",
+			name:              "rename non-scalar key",
-			in:          "foo:\n  bar: 5\n",
+			in:                "foo:\n  bar: 5\n",
-			path:        []string{"foo"},
+			path:              []string{"foo"},
-			newKey:      "qux",
+			newKey:            "qux",
-			expectedOut: "qux:\n  bar: 5\n",
+			expectedOut:       "qux:\n  bar: 5\n",
-			expectedErr: "",
+			expectedDidRename: true,
+			expectedErr:       "",
 		},
 		{
-			name:        "don't rewrite file if value didn't change",
+			name:              "don't rewrite file if value didn't change",
-			in:          "foo:\n  bar: 5\n",
+			in:                "foo:\n  bar: 5\n",
-			path:        []string{"nonExistingKey"},
+			path:              []string{"nonExistingKey"},
-			newKey:      "qux",
+			newKey:            "qux",
-			expectedOut: "foo:\n  bar: 5\n",
+			expectedOut:       "foo:\n  bar: 5\n",
-			expectedErr: "",
+			expectedDidRename: false,
+			expectedErr:       "",
 		},
 
 		// Error cases
 		{
-			name:        "existing document is not a dictionary",
+			name:              "existing document is not a dictionary",
-			in:          "42\n",
+			in:                "42\n",
-			path:        []string{"foo"},
+			path:              []string{"foo"},
-			newKey:      "bar",
+			newKey:            "bar",
-			expectedOut: "42\n",
+			expectedOut:       "42\n",
-			expectedErr: "yaml node in path is not a dictionary",
+			expectedDidRename: false,
+			expectedErr:       "yaml node in path is not a dictionary",
 		},
 		{
-			name:        "not all path elements are dictionaries",
+			name:              "not all path elements are dictionaries",
-			in:          "foo:\n  bar: [1, 2, 3]\n",
+			in:                "foo:\n  bar: [1, 2, 3]\n",
-			path:        []string{"foo", "bar", "baz"},
+			path:              []string{"foo", "bar", "baz"},
-			newKey:      "qux",
+			newKey:            "qux",
-			expectedOut: "foo:\n  bar: [1, 2, 3]\n",
+			expectedOut:       "foo:\n  bar: [1, 2, 3]\n",
-			expectedErr: "yaml node in path is not a dictionary",
+			expectedDidRename: false,
+			expectedErr:       "yaml node in path is not a dictionary",
 		},
 		{
-			name:        "new key exists",
+			name:              "new key exists",
-			in:          "foo: 5\nbar: 7\n",
+			in:                "foo: 5\nbar: 7\n",
-			path:        []string{"foo"},
+			path:              []string{"foo"},
-			newKey:      "bar",
+			newKey:            "bar",
-			expectedOut: "foo: 5\nbar: 7\n",
+			expectedOut:       "foo: 5\nbar: 7\n",
-			expectedErr: "new key `bar' already exists",
+			expectedDidRename: false,
+			expectedErr:       "new key `bar' already exists",
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			node := unmarshalForTest(t, test.in)
-			actualErr := RenameYamlKey(&node, test.path, test.newKey)
+			actualErr, didRename := RenameYamlKey(&node, test.path, test.newKey)
 			if test.expectedErr == "" {
 				assert.NoError(t, actualErr)
 			} else {
@@ -89,6 +97,8 @@ func TestRenameYamlKey(t *testing.T) {
 			out := marshalForTest(t, &node)
 
 			assert.Equal(t, test.expectedOut, out)
+
+			assert.Equal(t, test.expectedDidRename, didRename)
 		})
 	}
 }