From ecf8264ac7c6faf8169cd2df0693ed325dac7720 Mon Sep 17 00:00:00 2001 From: Shaun Duncan Date: Fri, 25 Oct 2013 16:44:32 -0400 Subject: [PATCH 1/2] Fixed inadvertent recursion in SelectNodes. Updated/add tests for this behavior --- node.go | 14 ++++++-------- xmlx_test.go | 31 +++++++++++++++++++++++++++---- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/node.go b/node.go index 5327b39..873cedd 100644 --- a/node.go +++ b/node.go @@ -383,15 +383,13 @@ func (this *Node) SelectNodesRecursive(namespace, name string) []*Node { } func rec_SelectNodes(cn *Node, namespace, name string, list *[]*Node, recurse bool) { - if (namespace == "*" || cn.Name.Space == namespace) && (name == "*" || cn.Name.Local == name) { - *list = append(*list, cn) - if !recurse { - return - } - } - for _, v := range cn.Children { - rec_SelectNodes(v, namespace, name, list, recurse) + if (namespace == "*" || v.Name.Space == namespace) && (name == "*" || v.Name.Local == name) { + *list = append(*list, v) + } + if recurse { + rec_SelectNodes(v, namespace, name, list, recurse) + } } } diff --git a/xmlx_test.go b/xmlx_test.go index 5dd8b82..201061d 100644 --- a/xmlx_test.go +++ b/xmlx_test.go @@ -28,10 +28,10 @@ func TestWildcard(t *testing.T) { return } - list := doc.SelectNodes("ns", "*") + list := doc.SelectNodesRecursive("ns", "*") - if len(list) != 1 { - t.Errorf("Wrong number of child elements. Expected 1, got %d.", len(list)) + if len(list) != 7 { + t.Errorf("Wrong number of child elements. Expected 7, got %d.", len(list)) return } } @@ -94,13 +94,36 @@ func TestNodeSearch(t *testing.T) { return } - nodes := doc.SelectNodes("", "item") + nodes := doc.SelectNodesRecursive("", "item") if len(nodes) == 0 { t.Errorf("SelectNodes(): no nodes found.") return } } +func TestSelectNodes(t *testing.T) { + doc := New() + + if err := doc.LoadFile("test1.xml", nil); err != nil { + t.Errorf("LoadFile(): %s", err) + return + } + + ch := doc.SelectNode("", "channel") + + topLevelLinks := ch.SelectNodes("", "link") + if len(topLevelLinks) != 1 { + t.Errorf("SelectNodes(): Expected 1, Got %d", len(topLevelLinks)) + return + } + + allLinks := ch.SelectNodesRecursive("", "link") + if len(allLinks) != 8 { + t.Errorf("SelectNodes(): Expected 8, Got %d", len(allLinks)) + return + } +} + type Image struct { Title string `xml:"title"` Url string `xml:"url"` From 72c0afcf9330bae7bc324513ced7372ca0790414 Mon Sep 17 00:00:00 2001 From: Shaun Duncan Date: Fri, 25 Oct 2013 17:31:10 -0400 Subject: [PATCH 2/2] Reworked the tests just a bit --- xmlx_test.go | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/xmlx_test.go b/xmlx_test.go index 201061d..d2c863e 100644 --- a/xmlx_test.go +++ b/xmlx_test.go @@ -28,10 +28,9 @@ func TestWildcard(t *testing.T) { return } - list := doc.SelectNodesRecursive("ns", "*") - - if len(list) != 7 { - t.Errorf("Wrong number of child elements. Expected 7, got %d.", len(list)) + list := doc.SelectNode("", "xml").SelectNodes("ns", "*") + if len(list) != 1 { + t.Errorf("Wrong number of child elements. Expected 1, got %d.", len(list)) return } } @@ -99,27 +98,19 @@ func TestNodeSearch(t *testing.T) { t.Errorf("SelectNodes(): no nodes found.") return } -} - -func TestSelectNodes(t *testing.T) { - doc := New() - - if err := doc.LoadFile("test1.xml", nil); err != nil { - t.Errorf("LoadFile(): %s", err) - return - } ch := doc.SelectNode("", "channel") - - topLevelLinks := ch.SelectNodes("", "link") - if len(topLevelLinks) != 1 { - t.Errorf("SelectNodes(): Expected 1, Got %d", len(topLevelLinks)) + // Test that SelectNodes doesn't accidentally do recursive + links := ch.SelectNodes("", "link") + if len(links) != 1 { + t.Errorf("SelectNodes(): Expected 1, Got %d", len(links)) return } - allLinks := ch.SelectNodesRecursive("", "link") - if len(allLinks) != 8 { - t.Errorf("SelectNodes(): Expected 8, Got %d", len(allLinks)) + // Test SelectNodesRecursive does indeed get all of them + links = ch.SelectNodesRecursive("", "link") + if len(links) != 8 { + t.Errorf("SelectNodesRecursive(): Expected 8, Got %d", len(links)) return } }