Skip to content

Commit

Permalink
DOM: Reverse order of nodes in coalesce
Browse files Browse the repository at this point in the history
Overriden value should be evaluated first, otherwise override will never work.
Move coalesce to appropriate merge*.go files and add unit test for this situation

Signed-off-by: Richard Kosegi <richard.kosegi@gmail.com>
  • Loading branch information
rkosegi committed Jul 19, 2024
1 parent 4241e28 commit 3593365
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 16 deletions.
15 changes: 14 additions & 1 deletion dom/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ limitations under the License.

package dom

import "math"
import (
"math"
"slices"
)

func defaultListMerger() MergeOption {
return func(m *merger) {
Expand Down Expand Up @@ -121,3 +124,13 @@ func (mg *merger) mergeOverlay(m *overlayDocument) Container {
}
return merged
}

func coalesce(nodes ...Node) Node {
slices.Reverse(nodes)
for _, node := range nodes {
if hasValue(node) {
return node
}
}
return nilLeaf
}
30 changes: 30 additions & 0 deletions dom/merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package dom

import (
"github.com/stretchr/testify/assert"
"strings"
"testing"
)

Expand Down Expand Up @@ -71,3 +72,32 @@ func TestMergeContainerFromTwoLists(t *testing.T) {
l := m.mergeListsMeld(ListNode(c1), ListNode(c2))
assert.Equal(t, 1, l.Size())
}

func TestCoalesce(t *testing.T) {
assert.Equal(t, nilLeaf, coalesce(nilLeaf))
assert.Equal(t, 123, coalesce(nilLeaf,
LeafNode(123), nilLeaf).(Leaf).Value())
}

func TestMergeOverrideLeafValue(t *testing.T) {
d1 := `
---
root:
sub1:
sub2:
leaf: 1
`
d2 := `
---
root:
sub1:
sub2:
leaf: 2
`
orig, err := b.FromReader(strings.NewReader(d1), DefaultYamlDecoder)
assert.NoError(t, err)
override, err := b.FromReader(strings.NewReader(d2), DefaultYamlDecoder)
assert.NoError(t, err)
result := orig.Merge(override)
assert.Equal(t, 2, result.Lookup("root.sub1.sub2.leaf").(Leaf).Value())
}
9 changes: 0 additions & 9 deletions dom/overlay.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,15 +175,6 @@ func hasValue(n Node) bool {
return true
}

func coalesce(nodes ...Node) Node {
for _, node := range nodes {
if hasValue(node) {
return node
}
}
return nilLeaf
}

func firstValidListItem(idx int, lists ...List) Node {
for _, list := range lists {
if list.Size() > idx {
Expand Down
6 changes: 0 additions & 6 deletions dom/overlay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,6 @@ func TestLoadLookupList(t *testing.T) {
assert.Equal(t, "hello", n.(Leaf).Value())
}

func TestCoalesce(t *testing.T) {
assert.Equal(t, nilLeaf, coalesce(nilLeaf))
assert.Equal(t, 123, coalesce(nilLeaf,
LeafNode(123), nilLeaf).(Leaf).Value())
}

func TestFirstValidListItem(t *testing.T) {
assert.Equal(t, 456, firstValidListItem(1,
ListNode(),
Expand Down

0 comments on commit 3593365

Please sign in to comment.