From bf95068b11777686b7a2967946eba78a8cece498 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 7 May 2025 18:12:25 +0200 Subject: [PATCH] Make RenameYamlKey return a bool --- pkg/config/app_config.go | 2 +- pkg/utils/yaml_utils/yaml_utils.go | 14 +-- pkg/utils/yaml_utils/yaml_utils_test.go | 108 +++++++++++++----------- 3 files changed, 67 insertions(+), 57 deletions(-) diff --git a/pkg/config/app_config.go b/pkg/config/app_config.go index ae1b0d38e..010db02ef 100644 --- a/pkg/config/app_config.go +++ b/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) } diff --git a/pkg/utils/yaml_utils/yaml_utils.go b/pkg/utils/yaml_utils/yaml_utils.go index 5f2532a3d..432915f5e 100644 --- a/pkg/utils/yaml_utils/yaml_utils.go +++ b/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) diff --git a/pkg/utils/yaml_utils/yaml_utils_test.go b/pkg/utils/yaml_utils/yaml_utils_test.go index b49130ea7..9ae099396 100644 --- a/pkg/utils/yaml_utils/yaml_utils_test.go +++ b/pkg/utils/yaml_utils/yaml_utils_test.go @@ -10,77 +10,85 @@ import ( func TestRenameYamlKey(t *testing.T) { tests := []struct { - name string - in string - path []string - newKey string - expectedOut string - expectedErr string + name string + in string + path []string + newKey string + expectedOut string + expectedDidRename bool + expectedErr string }{ { - name: "rename key", - in: "foo: 5\n", - path: []string{"foo"}, - newKey: "bar", - expectedOut: "bar: 5\n", - expectedErr: "", + name: "rename key", + in: "foo: 5\n", + path: []string{"foo"}, + newKey: "bar", + expectedOut: "bar: 5\n", + expectedDidRename: true, + expectedErr: "", }, { - name: "rename key, nested", - in: "foo:\n bar: 5\n", - path: []string{"foo", "bar"}, - newKey: "baz", - expectedOut: "foo:\n baz: 5\n", - expectedErr: "", + name: "rename key, nested", + in: "foo:\n bar: 5\n", + path: []string{"foo", "bar"}, + newKey: "baz", + expectedOut: "foo:\n baz: 5\n", + expectedDidRename: true, + expectedErr: "", }, { - name: "rename non-scalar key", - in: "foo:\n bar: 5\n", - path: []string{"foo"}, - newKey: "qux", - expectedOut: "qux:\n bar: 5\n", - expectedErr: "", + name: "rename non-scalar key", + in: "foo:\n bar: 5\n", + path: []string{"foo"}, + newKey: "qux", + expectedOut: "qux:\n bar: 5\n", + expectedDidRename: true, + expectedErr: "", }, { - name: "don't rewrite file if value didn't change", - in: "foo:\n bar: 5\n", - path: []string{"nonExistingKey"}, - newKey: "qux", - expectedOut: "foo:\n bar: 5\n", - expectedErr: "", + name: "don't rewrite file if value didn't change", + in: "foo:\n bar: 5\n", + path: []string{"nonExistingKey"}, + newKey: "qux", + expectedOut: "foo:\n bar: 5\n", + expectedDidRename: false, + expectedErr: "", }, // Error cases { - name: "existing document is not a dictionary", - in: "42\n", - path: []string{"foo"}, - newKey: "bar", - expectedOut: "42\n", - expectedErr: "yaml node in path is not a dictionary", + name: "existing document is not a dictionary", + in: "42\n", + path: []string{"foo"}, + newKey: "bar", + expectedOut: "42\n", + expectedDidRename: false, + expectedErr: "yaml node in path is not a dictionary", }, { - name: "not all path elements are dictionaries", - in: "foo:\n bar: [1, 2, 3]\n", - path: []string{"foo", "bar", "baz"}, - newKey: "qux", - expectedOut: "foo:\n bar: [1, 2, 3]\n", - expectedErr: "yaml node in path is not a dictionary", + name: "not all path elements are dictionaries", + in: "foo:\n bar: [1, 2, 3]\n", + path: []string{"foo", "bar", "baz"}, + newKey: "qux", + expectedOut: "foo:\n bar: [1, 2, 3]\n", + expectedDidRename: false, + expectedErr: "yaml node in path is not a dictionary", }, { - name: "new key exists", - in: "foo: 5\nbar: 7\n", - path: []string{"foo"}, - newKey: "bar", - expectedOut: "foo: 5\nbar: 7\n", - expectedErr: "new key `bar' already exists", + name: "new key exists", + in: "foo: 5\nbar: 7\n", + path: []string{"foo"}, + newKey: "bar", + expectedOut: "foo: 5\nbar: 7\n", + 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) }) } }