From 30845e3d659e64077881257ddc10634b83d40bff Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Tue, 1 Jul 2014 09:57:43 +0200
Subject: [PATCH] Reliably remove nodes from owned sets.

The combination of iterating over an array and removing values from it results
in not all elements being removed. For example:

    numbers = [10, 20, 30]

    numbers.each do |num|
      numbers.delete(num)
    end

    numbers # => [20]

As a result of this the NodeSet#remove method uses two iterations:

1. One iteration to retrieve all NodeSet instances to remove nodes from.
2. One iteration to actually remove the nodes.

For the second iteration we iterate over the node sets and then over the nodes.
This ensures that we always remove all nodes instead of leaving some behind.
---
 lib/oga/xml/node_set.rb       | 16 ++++++++++++++--
 spec/oga/xml/node_set_spec.rb |  6 ++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/lib/oga/xml/node_set.rb b/lib/oga/xml/node_set.rb
index 2b409d5..d50f193 100644
--- a/lib/oga/xml/node_set.rb
+++ b/lib/oga/xml/node_set.rb
@@ -166,9 +166,21 @@ module Oga
       # This method is intended to remove nodes from an XML document/node.
       #
       def remove
+        sets = []
+
+        # First we gather all the sets to remove nodse from, then we remove the
+        # actual nodes. This is done as you can not reliably remove elements
+        # from an Array while iterating on that same Array.
         @nodes.each do |node|
-          node.node_set.delete(node)
-          node.node_set = nil
+          if node.node_set
+            sets << node.node_set
+
+            node.node_set = nil
+          end
+        end
+
+        sets.each do |set|
+          @nodes.each { |node| set.delete(node) }
         end
       end
 
diff --git a/spec/oga/xml/node_set_spec.rb b/spec/oga/xml/node_set_spec.rb
index 6945bb9..14f6ab9 100644
--- a/spec/oga/xml/node_set_spec.rb
+++ b/spec/oga/xml/node_set_spec.rb
@@ -213,6 +213,12 @@ describe Oga::XML::NodeSet do
       @n1.node_set.nil?.should == true
       @n2.node_set.nil?.should == true
     end
+
+    example 'remove all nodes from the owned set' do
+      @doc_set.remove
+
+      @doc_set.empty?.should == true
+    end
   end
 
   context '#delete' do