RemoveChildAt sometimes dosent work

[speatzle]

Hi, i have been trying to add and delete rules from a pfsense XML config. Adding has been working great but deleting using RemoveChildAt doesn't work sometimes.

Here is my example code:

package main

import (
	"fmt"

	"github.com/beevik/etree"
)

const xml = `
<pfsense>
    <filter>
        <rule>
            <created>
                <username>testUser</username>
            </created>
        </rule>
        <rule>
            <created>
                <username>testUser</username>
            </created>
        </rule>
        <rule>
            <created>
                <username>admin</username>
            </created>
        </rule>
        <rule>
            <created>
                <username>testUser</username>
            </created>
        </rule>
        <rule>
            <created>
                <username>admin</username>
            </created>
        </rule>
    </filter>
</pfsense>
`

func main() {
	fmt.Println("Starting!")

	doc := etree.NewDocument()
	err := doc.ReadFromString(xml)
	if err != nil {
		panic(err)
	}

	fmt.Println("Original Rules:")
	displayRules(doc)

	fmt.Println("Deleting old Rules:")
	deleteOldRules(doc)

	doc.Indent(4)

	fmt.Println("New Rules:")

	displayRules(doc)
}

func displayRules(doc *etree.Document) {
	for _, rule := range doc.FindElements("pfsense/filter/rule") {
		fmt.Println("	Rule Element:", rule.Tag)
		username := rule.FindElement("created/username")
		if username != nil {
			fmt.Println("		Username: ", username.Text())
		}
	}
}

func deleteOldRules(doc *etree.Document) {
	oldrules := []int{}
	for i, rule := range doc.FindElements("pfsense/filter/*") {
		if rule.Tag == "rule" {
			username := rule.FindElement("created/username")
			if username == nil {
				//rule has no user
				continue
			}
			if username.Text() == "testUser" {
				fmt.Println("Old rule at index: ", i)
				oldrules = append([]int{i}, oldrules...)
			}
		}
	}
	for _, i := range oldrules {
		fmt.Println("deleting index", i)
		doc.FindElement("pfsense/filter").RemoveChildAt(i)
	}
}

And this is the output i get:

Starting!
Original Rules:
        Rule Element: rule
                Username:  testUser
        Rule Element: rule
                Username:  testUser
        Rule Element: rule
                Username:  admin
        Rule Element: rule
                Username:  testUser
        Rule Element: rule
                Username:  admin
Deleting old Rules:
Old rule at index:  0
Old rule at index:  1
Old rule at index:  3
deleting index 3
deleting index 1
deleting index 0
New Rules:
        Rule Element: rule
                Username:  admin
        Rule Element: rule
                Username:  testUser
        Rule Element: rule
                Username:  admin

What this code should do is go through all rule elements in the filter element and see if that rule has a "created" element with a child "username" element with the text "testUser". if this is true then write down the index of the rule into a slice . This part works fine As it detected all 3 rules with the indexes 0,1,3.
Then it tries to delete the rules by index. The loop works fine as it first tries to delete index 3 then 1 and 0.
But when i then loop over all rules again the last rule element is still there.

[beevik]

I think the problem is that removing elements modifies the indices of the remaining elements. Have you tried deleting in reverse order? That would probably work, because the indices of your removed elements won't be changing as you iterate.

[speatzle]

If you look at my code then you can see that i am prepending elements to the deletion list
oldrules = append([]int{i}, oldrules...)

And if look at my log output it also counts backwards while deleting. (first it detects the rules then deletes them in reverse order)

Old rule at index:  0
Old rule at index:  1
Old rule at index:  3
deleting index 3
deleting index 1
deleting index 0

In my explanation i also State that it first tries to delete the third then the first and lastly the zeroth element.

[speatzle]

So i have made a simplified version without the loop.

const xml = `
<pfsense>
    <filter>
        <rule>
            <created>
                <username>testUser</username>
            </created>
        </rule>
        <rule>
            <created>
                <username>testUser</username>
            </created>
        </rule>
        <rule>
            <created>
                <username>admin</username>
            </created>
        </rule>
        <rule>
            <created>
                <username>testUser</username>
            </created>
        </rule>
        <rule>
            <created>
                <username>admin</username>
            </created>
		</rule>
    </filter>
</pfsense>
`

func main() {
	fmt.Println("Starting!")

	doc := etree.NewDocument()
	err := doc.ReadFromString(xml)
	if err != nil {
		panic(err)
	}

	fmt.Println("Original Rules:")
	displayRules(doc)

	fmt.Println("Deleting old Rules:")
	doc.FindElement("pfsense/filter").RemoveChildAt(3)
	doc.FindElement("pfsense/filter").RemoveChildAt(1)
	doc.FindElement("pfsense/filter").RemoveChildAt(0)

	doc.Indent(4)

	fmt.Println("New Rules:")

	displayRules(doc)
}

func displayRules(doc *etree.Document) {
	for _, rule := range doc.FindElements("pfsense/filter/rule") {
		fmt.Println("	Rule Element:", rule.Tag)
		username := rule.FindElement("created/username")
		if username != nil {
			fmt.Println("		Username: ", username.Text())
		}
	}
}

So when i runt this updated code now i get the same output as before.

[beevik]

Ok thanks for the further testing. Looks like a probable bug. I'll take a look today.

[beevik]

Ah, so the problem is that the RemoveChildAt function removes the n-th child token, not the n-th child element. Tokens include elements, but they also include character data (e.g., white space between elements). In your latest example, the token at index 0 is the white space between <filter> and <rule>, the token at index 1 is the XML element for the first testUser, and the token at index 3 is the XML element for the second testUser.

To do what you're trying to do, you'll need to process the child elements of the <filter> element using something like this:

filter := doc.FindElement("pfsense/filter")
rules := filter.SelectElements("rule")
filter.RemoveChildAt(rules[3].Index())
filter.RemoveChildAt(rules[1].Index())
filter.RemoveChildAt(rules[0].Index())

(Note that I'm not doing any bounds checking on the rules here. This is just an example.)

[speatzle]

Oh, interesting. I didn't even think that Children could be anything other than elements. Well, thank you.