From 9de257300984c39cd3b72ddac8fd411db0420c75 Mon Sep 17 00:00:00 2001 From: Mike Farah Date: Fri, 7 Feb 2020 16:32:39 +1100 Subject: [PATCH] Fixed merge append arrays --- cmd/commands_test.go | 27 +++++++++++++++++++++++++ cmd/merge.go | 13 +++++++++++- cmd/utils.go | 22 +++++++++++++++----- pkg/yqlib/data_navigator.go | 8 ++++++-- pkg/yqlib/delete_navigation_strategy.go | 3 +++ pkg/yqlib/lib.go | 6 +++--- pkg/yqlib/navigation_strategy.go | 19 ++++++++++------- pkg/yqlib/read_navigation_strategy.go | 15 +++++++++++++- pkg/yqlib/update_navigation_strategy.go | 3 +++ 9 files changed, 97 insertions(+), 19 deletions(-) diff --git a/cmd/commands_test.go b/cmd/commands_test.go index 5233fb6..4253fa5 100644 --- a/cmd/commands_test.go +++ b/cmd/commands_test.go @@ -1812,6 +1812,33 @@ c: test.AssertResult(t, expectedOutput, result.Output) } +func TestMergeAppendArraysCmd(t *testing.T) { + content := `people: + - name: Barry + age: 21` + filename := test.WriteTempYamlFile(content) + defer test.RemoveTempYamlFile(filename) + + mergeContent := `people: + - name: Roger + age: 44` + mergeFilename := test.WriteTempYamlFile(mergeContent) + defer test.RemoveTempYamlFile(mergeFilename) + + cmd := getRootCommand() + result := test.RunCmd(cmd, fmt.Sprintf("merge --append -d* %s %s", filename, mergeFilename)) + if result.Error != nil { + t.Error(result.Error) + } + expectedOutput := `people: +- name: Barry + age: 21 +- name: Roger + age: 44 +` + test.AssertResult(t, expectedOutput, result.Output) +} + func TestMergeOverwriteAndAppendCmd(t *testing.T) { cmd := getRootCommand() result := test.RunCmd(cmd, "merge --autocreate=false --append --overwrite ../examples/data1.yaml ../examples/data2.yaml") diff --git a/cmd/merge.go b/cmd/merge.go index 645eb30..d13b608 100644 --- a/cmd/merge.go +++ b/cmd/merge.go @@ -4,6 +4,7 @@ import ( "github.com/mikefarah/yq/v3/pkg/yqlib" errors "github.com/pkg/errors" "github.com/spf13/cobra" + yaml "gopkg.in/yaml.v3" ) func createMergeCmd() *cobra.Command { @@ -36,6 +37,16 @@ If append flag is set then existing arrays will be merged with the arrays from e return cmdMerge } +/* +* We don't deeply traverse arrays when appending a merge, instead we want to +* append the entire array element. + */ +func createReadFunctionForMerge() func(*yaml.Node) ([]*yqlib.NodeContext, error) { + return func(dataBucket *yaml.Node) ([]*yqlib.NodeContext, error) { + return lib.Get(dataBucket, "**", !appendFlag) + } +} + func mergeProperties(cmd *cobra.Command, args []string) error { var updateCommands []yqlib.UpdateCommand = make([]yqlib.UpdateCommand, 0) @@ -48,7 +59,7 @@ func mergeProperties(cmd *cobra.Command, args []string) error { var filesToMerge = args[1:] for _, fileToMerge := range filesToMerge { - matchingNodes, errorProcessingFile := readYamlFile(fileToMerge, "**", false, 0) + matchingNodes, errorProcessingFile := doReadYamlFile(fileToMerge, createReadFunctionForMerge(), false, 0) if errorProcessingFile != nil { return errorProcessingFile } diff --git a/cmd/utils.go b/cmd/utils.go index 791be2d..1d491f2 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -13,7 +13,19 @@ import ( yaml "gopkg.in/yaml.v3" ) +type readDataFn func(dataBucket *yaml.Node) ([]*yqlib.NodeContext, error) + +func createReadFunction(path string) func(*yaml.Node) ([]*yqlib.NodeContext, error) { + return func(dataBucket *yaml.Node) ([]*yqlib.NodeContext, error) { + return lib.Get(dataBucket, path, true) + } +} + func readYamlFile(filename string, path string, updateAll bool, docIndexInt int) ([]*yqlib.NodeContext, error) { + return doReadYamlFile(filename, createReadFunction(path), updateAll, docIndexInt) +} + +func doReadYamlFile(filename string, readFn readDataFn, updateAll bool, docIndexInt int) ([]*yqlib.NodeContext, error) { var matchingNodes []*yqlib.NodeContext var currentIndex = 0 @@ -29,7 +41,7 @@ func readYamlFile(filename string, path string, updateAll bool, docIndexInt int) } var errorParsing error - matchingNodes, errorParsing = appendDocument(matchingNodes, dataBucket, path, updateAll, docIndexInt, currentIndex) + matchingNodes, errorParsing = appendDocument(matchingNodes, dataBucket, readFn, updateAll, docIndexInt, currentIndex) if errorParsing != nil { return errorParsing } @@ -51,14 +63,14 @@ func handleEOF(updateAll bool, docIndexInt int, currentIndex int) error { return nil } -func appendDocument(originalMatchingNodes []*yqlib.NodeContext, dataBucket yaml.Node, path string, updateAll bool, docIndexInt int, currentIndex int) ([]*yqlib.NodeContext, error) { +func appendDocument(originalMatchingNodes []*yqlib.NodeContext, dataBucket yaml.Node, readFn readDataFn, updateAll bool, docIndexInt int, currentIndex int) ([]*yqlib.NodeContext, error) { log.Debugf("processing document %v - requested index %v", currentIndex, docIndexInt) yqlib.DebugNode(&dataBucket) if !updateAll && currentIndex != docIndexInt { return originalMatchingNodes, nil } - log.Debugf("reading %v in document %v", path, currentIndex) - matchingNodes, errorParsing := lib.Get(&dataBucket, path) + log.Debugf("reading in document %v", currentIndex) + matchingNodes, errorParsing := readFn(&dataBucket) if errorParsing != nil { return nil, errors.Wrapf(errorParsing, "Error reading path in document index %v", currentIndex) } @@ -106,7 +118,7 @@ func explode(matchingNodes []*yqlib.NodeContext) error { for _, nodeContext := range matchingNodes { var targetNode = yaml.Node{Kind: yaml.MappingNode} - explodedNodes, errorRetrieving := lib.Get(nodeContext.Node, "**") + explodedNodes, errorRetrieving := lib.Get(nodeContext.Node, "**", true) if errorRetrieving != nil { return errorRetrieving } diff --git a/pkg/yqlib/data_navigator.go b/pkg/yqlib/data_navigator.go index 0370cd3..98c78e9 100644 --- a/pkg/yqlib/data_navigator.go +++ b/pkg/yqlib/data_navigator.go @@ -34,9 +34,10 @@ func (n *navigator) doTraverse(value *yaml.Node, head string, tail []string, pat log.Debug("head %v", head) DebugNode(value) + var nodeContext = NewNodeContext(value, head, tail, pathStack) var errorDeepSplatting error - if head == "**" && value.Kind != yaml.ScalarNode { + if head == "**" && value.Kind != yaml.ScalarNode && n.navigationStrategy.ShouldDeeplyTraverse(nodeContext) { if len(pathStack) == 0 || pathStack[len(pathStack)-1] != "<<" { errorDeepSplatting = n.recurse(value, head, tail, pathStack) } @@ -53,7 +54,7 @@ func (n *navigator) doTraverse(value *yaml.Node, head string, tail []string, pat DebugNode(value) return n.recurse(value, tail[0], tail[1:], pathStack) } - return n.navigationStrategy.Visit(NewNodeContext(value, head, tail, pathStack)) + return n.navigationStrategy.Visit(nodeContext) } func (n *navigator) getOrReplace(original *yaml.Node, expectedKind yaml.Kind) *yaml.Node { @@ -221,6 +222,9 @@ func (n *navigator) splatArray(value *yaml.Node, head string, tail []string, pat newPathStack := append(pathStack, index) if n.navigationStrategy.ShouldTraverse(NewNodeContext(childValue, head, tail, newPathStack), childValue.Value) { + // here we should not deeply traverse the array if we are appending..not sure how to do that. + // need to visit instead... + // easiest way is to pop off the head and pass the rest of the tail in. var err = n.doTraverse(childValue, head, tail, newPathStack) if err != nil { return err diff --git a/pkg/yqlib/delete_navigation_strategy.go b/pkg/yqlib/delete_navigation_strategy.go index cff9a42..4520bef 100644 --- a/pkg/yqlib/delete_navigation_strategy.go +++ b/pkg/yqlib/delete_navigation_strategy.go @@ -17,6 +17,9 @@ func DeleteNavigationStrategy(pathElementToDelete string) NavigationStrategy { autoCreateMap: func(nodeContext NodeContext) bool { return false }, + shouldDeeplyTraverse: func(nodeContext NodeContext) bool { + return true + }, visit: func(nodeContext NodeContext) error { node := nodeContext.Node log.Debug("need to find and delete %v in here", pathElementToDelete) diff --git a/pkg/yqlib/lib.go b/pkg/yqlib/lib.go index f4f3889..612a80e 100644 --- a/pkg/yqlib/lib.go +++ b/pkg/yqlib/lib.go @@ -97,7 +97,7 @@ func guessKind(head string, tail []string, guess yaml.Kind) yaml.Kind { } type YqLib interface { - Get(rootNode *yaml.Node, path string) ([]*NodeContext, error) + Get(rootNode *yaml.Node, path string, deeplyTraverseArrays bool) ([]*NodeContext, error) Update(rootNode *yaml.Node, updateCommand UpdateCommand, autoCreate bool) error New(path string) yaml.Node @@ -115,9 +115,9 @@ func NewYqLib() YqLib { } } -func (l *lib) Get(rootNode *yaml.Node, path string) ([]*NodeContext, error) { +func (l *lib) Get(rootNode *yaml.Node, path string, deeplyTraverseArrays bool) ([]*NodeContext, error) { var paths = l.parser.ParsePath(path) - navigationStrategy := ReadNavigationStrategy() + navigationStrategy := ReadNavigationStrategy(deeplyTraverseArrays) navigator := NewDataNavigator(navigationStrategy) error := navigator.Traverse(rootNode, paths) return navigationStrategy.GetVisitedNodes(), error diff --git a/pkg/yqlib/navigation_strategy.go b/pkg/yqlib/navigation_strategy.go index 892988a..03bdaf4 100644 --- a/pkg/yqlib/navigation_strategy.go +++ b/pkg/yqlib/navigation_strategy.go @@ -34,18 +34,20 @@ type NavigationStrategy interface { // node key is the string value of the last element in the path stack // we use it to match against the pathExpression in head. ShouldTraverse(nodeContext NodeContext, nodeKey string) bool + ShouldDeeplyTraverse(nodeContext NodeContext) bool GetVisitedNodes() []*NodeContext DebugVisitedNodes() GetPathParser() PathParser } type NavigationStrategyImpl struct { - followAlias func(nodeContext NodeContext) bool - autoCreateMap func(nodeContext NodeContext) bool - visit func(nodeContext NodeContext) error - shouldVisitExtraFn func(nodeContext NodeContext) bool - visitedNodes []*NodeContext - pathParser PathParser + followAlias func(nodeContext NodeContext) bool + autoCreateMap func(nodeContext NodeContext) bool + visit func(nodeContext NodeContext) error + shouldVisitExtraFn func(nodeContext NodeContext) bool + shouldDeeplyTraverse func(nodeContext NodeContext) bool + visitedNodes []*NodeContext + pathParser PathParser } func (ns *NavigationStrategyImpl) GetPathParser() PathParser { @@ -64,6 +66,10 @@ func (ns *NavigationStrategyImpl) AutoCreateMap(nodeContext NodeContext) bool { return ns.autoCreateMap(nodeContext) } +func (ns *NavigationStrategyImpl) ShouldDeeplyTraverse(nodeContext NodeContext) bool { + return ns.shouldDeeplyTraverse(nodeContext) +} + func (ns *NavigationStrategyImpl) ShouldTraverse(nodeContext NodeContext, nodeKey string) bool { // we should traverse aliases (if enabled), but not visit them :/ if len(nodeContext.PathStack) == 0 { @@ -84,7 +90,6 @@ func (ns *NavigationStrategyImpl) shouldVisit(nodeContext NodeContext) bool { return true } log.Debug("tail len %v", len(nodeContext.Tail)) - // SOMETHING HERE! if ns.alreadyVisited(pathStack) || len(nodeContext.Tail) != 0 { return false diff --git a/pkg/yqlib/read_navigation_strategy.go b/pkg/yqlib/read_navigation_strategy.go index 4040e50..0e02d52 100644 --- a/pkg/yqlib/read_navigation_strategy.go +++ b/pkg/yqlib/read_navigation_strategy.go @@ -1,6 +1,6 @@ package yqlib -func ReadNavigationStrategy() NavigationStrategy { +func ReadNavigationStrategy(deeplyTraverseArrays bool) NavigationStrategy { return &NavigationStrategyImpl{ visitedNodes: []*NodeContext{}, pathParser: NewPathParser(), @@ -13,5 +13,18 @@ func ReadNavigationStrategy() NavigationStrategy { visit: func(nodeContext NodeContext) error { return nil }, + shouldDeeplyTraverse: func(nodeContext NodeContext) bool { + var isInArray = false + if len(nodeContext.PathStack) > 0 { + var lastElement = nodeContext.PathStack[len(nodeContext.PathStack)-1] + switch lastElement.(type) { + case int: + isInArray = true + default: + isInArray = false + } + } + return deeplyTraverseArrays || !isInArray + }, } } diff --git a/pkg/yqlib/update_navigation_strategy.go b/pkg/yqlib/update_navigation_strategy.go index 9cb4ac9..1133742 100644 --- a/pkg/yqlib/update_navigation_strategy.go +++ b/pkg/yqlib/update_navigation_strategy.go @@ -10,6 +10,9 @@ func UpdateNavigationStrategy(updateCommand UpdateCommand, autoCreate bool) Navi autoCreateMap: func(nodeContext NodeContext) bool { return autoCreate }, + shouldDeeplyTraverse: func(nodeContext NodeContext) bool { + return true + }, visit: func(nodeContext NodeContext) error { node := nodeContext.Node changesToApply := updateCommand.Value