Skip to content

Commit

Permalink
prefactor: cleans up Document#root= tests and C code
Browse files Browse the repository at this point in the history
  • Loading branch information
@flavorjones
flavorjones committed Mar 22, 2021
1 parent eae58a9 commit 2920cf4
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 74 deletions.
67 changes: 31 additions & 36 deletions ext/nokogiri/xml_document.c
View file
Expand Up @@ -141,42 +141,35 @@ url(VALUE self)
* Set the root element on this document
*/
static VALUE
set_root(VALUE self, VALUE root)
rb_xml_document_root_set(VALUE self, VALUE rb_new_root)
{
xmlDocPtr doc;
xmlNodePtr new_root;
xmlNodePtr old_root;

Data_Get_Struct(self, xmlDoc, doc);
xmlDocPtr c_document;
xmlNodePtr c_new_root = NULL, c_current_root;

old_root = NULL;

if (NIL_P(root)) {
old_root = xmlDocGetRootElement(doc);

if (old_root) {
xmlUnlinkNode(old_root);
noko_xml_document_pin_node(old_root);
}
Data_Get_Struct(self, xmlDoc, c_document);

return root;
c_current_root = xmlDocGetRootElement(c_document);
if (c_current_root) {
xmlUnlinkNode(c_current_root);
noko_xml_document_pin_node(c_current_root);
}

Data_Get_Struct(root, xmlNode, new_root);
if (!NIL_P(rb_new_root)) {
Data_Get_Struct(rb_new_root, xmlNode, c_new_root);


/* If the new root's document is not the same as the current document,
* then we need to dup the node in to this document. */
if (new_root->doc != doc) {
old_root = xmlDocGetRootElement(doc);
if (!(new_root = xmlDocCopyNode(new_root, doc, 1))) {
rb_raise(rb_eRuntimeError, "Could not reparent node (xmlDocCopyNode)");
/* If the new root's document is not the same as the current document,
* then we need to dup the node in to this document. */
if (c_new_root->doc != c_document) {
c_new_root = xmlDocCopyNode(c_new_root, c_document, 1);
if (!c_new_root) {
rb_raise(rb_eRuntimeError, "Could not reparent node (xmlDocCopyNode)");
}
}
}

xmlDocSetRootElement(doc, new_root);
if (old_root) { noko_xml_document_pin_node(old_root); }
return root;
xmlDocSetRootElement(c_document, c_new_root);

return rb_new_root;
}

/*
Expand All @@ -186,17 +179,19 @@ set_root(VALUE self, VALUE root)
* Get the root node for this document.
*/
static VALUE
root(VALUE self)
rb_xml_document_root(VALUE self)
{
xmlDocPtr doc;
xmlNodePtr root;
xmlDocPtr c_document;
xmlNodePtr c_root;

Data_Get_Struct(self, xmlDoc, doc);
Data_Get_Struct(self, xmlDoc, c_document);

root = xmlDocGetRootElement(doc);
c_root = xmlDocGetRootElement(c_document);
if (!c_root) {
return Qnil;
}

if (!root) { return Qnil; }
return noko_xml_node_wrap(Qnil, root) ;
return noko_xml_node_wrap(Qnil, c_root) ;
}

/*
Expand Down Expand Up @@ -666,8 +661,8 @@ noko_init_xml_document()
rb_define_singleton_method(cNokogiriXmlDocument, "read_io", read_io, 4);
rb_define_singleton_method(cNokogiriXmlDocument, "new", new, -1);

rb_define_method(cNokogiriXmlDocument, "root", root, 0);
rb_define_method(cNokogiriXmlDocument, "root=", set_root, 1);
rb_define_method(cNokogiriXmlDocument, "root", rb_xml_document_root, 0);
rb_define_method(cNokogiriXmlDocument, "root=", rb_xml_document_root_set, 1);
rb_define_method(cNokogiriXmlDocument, "encoding", encoding, 0);
rb_define_method(cNokogiriXmlDocument, "encoding=", set_encoding, 1);
rb_define_method(cNokogiriXmlDocument, "version", version, 0);
Expand Down
79 changes: 41 additions & 38 deletions test/xml/test_document.rb
View file
Expand Up @@ -77,11 +77,6 @@ def test_document_with_initial_space
assert_equal(2, doc.children.size)
end

def test_root_set_to_nil
xml.root = nil
assert_nil(xml.root)
end

def test_million_laugh_attach
doc = Nokogiri::XML(<<~EOF)
<?xml version="1.0"?>
Expand Down Expand Up @@ -146,14 +141,15 @@ def test_collect_namespaces
end

def test_subclass_initialize_modify # testing a segv
Class.new(Nokogiri::XML::Document) do
doc = Class.new(Nokogiri::XML::Document) do
def initialize
super
body_node = Nokogiri::XML::Node.new("body", self)
body_node.content = "stuff"
self.root = body_node
end
end.new
assert_equal("stuff", doc.root.content)
end

def test_create_text_node
Expand Down Expand Up @@ -419,18 +415,6 @@ def test_prepend_child_with_string
assert_equal("quack!", doc.root.children.first.content)
end

def test_move_root_to_document_with_no_root
sender = Nokogiri::XML("<root>foo</root>")
newdoc = Nokogiri::XML::Document.new
newdoc.root = sender.root
end

def test_move_root_with_existing_root_gets_gcd
doc = Nokogiri::XML("<root>test</root>")
doc2 = Nokogiri::XML("<root>#{'x' * 5000000}</root>")
doc2.root = doc.root
end

def test_validate
if Nokogiri.uses_libxml?
assert_equal(45, xml.validate.length)
Expand Down Expand Up @@ -852,20 +836,6 @@ def test_new
assert_nil(doc.root)
end

def test_set_root
doc = nil
doc = Nokogiri::XML::Document.new
assert(doc)
assert(doc.xml?)
assert_nil(doc.root)
node = Nokogiri::XML::Node.new("b", doc) do |n|
n.content = "hello world"
end
assert_equal("hello world", node.content)
doc.root = node
assert_equal(node, doc.root)
end

def test_remove_namespaces
doc = Nokogiri::XML(<<~EOX)
<root xmlns:a="http://a.flavorjon.es/" xmlns:b="http://b.flavorjon.es/">
Expand Down Expand Up @@ -958,6 +928,12 @@ def awesome!
assert(xml.children.respond_to?(:awesome!))
end

def test_can_be_closed
f = File.open(XML_FILE)
Nokogiri::XML(f)
f.close
end

describe "JRuby-only methods" do
describe "Document.wrap" do
if Nokogiri.jruby?
Expand Down Expand Up @@ -1034,12 +1010,6 @@ def awesome!
end
end

def test_can_be_closed
f = File.open(XML_FILE)
Nokogiri::XML(f)
f.close
end

describe "XML::Document.parse" do
# establish baseline behavior for HTML document behavior in
# https://github.com/sparklemotion/nokogiri/issues/2130
Expand Down Expand Up @@ -1148,6 +1118,39 @@ def initialize(*args)
end
end
end

describe "#root=" do
it "assigns a root node" do
doc = Nokogiri::XML::Document.new
node = doc.create_element("div")
assert_nil(doc.root)

doc.root = node

assert_equal(node, doc.root)
end

it "allows removing the root node by setting to nil" do
assert_kind_of(Nokogiri::XML::Node, xml.root)
assert_nil(xml.root = nil)
assert_nil(xml.root)
end

it "supports moving nodes across documents" do
source = Nokogiri::XML("<root>foo</root>")
target = Nokogiri::XML::Document.new
assert_nil(target.root)
target.root = source.root
assert_equal("foo", target.root.content)
end

it "doesn't leak the replaced node" do
skip("only run if NOKOGIRI_GC is set") unless ENV['NOKOGIRI_GC']
doc = Nokogiri::XML("<root>test</root>")
doc2 = Nokogiri::XML("<root>#{'x' * 5000000}</root>")
doc2.root = doc.root
end
end
end
end
end
Expand Down

0 comments on commit 2920cf4

Please sign in to comment.