From 12874a7a6b43db2f75be96d9cc77fe426d5ec433 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Thu, 21 Jul 2022 12:21:45 -0400 Subject: [PATCH] feat: Reader#attribute_hash which is now being used to implement Reader#attributes fix: Reader#namespaces on JRuby now returns an empty hash when no attributes are present (previously returned nil) --- ext/java/nokogiri/XmlReader.java | 7 +++ ext/java/nokogiri/internals/ReaderNode.java | 5 +- ext/nokogiri/xml_reader.c | 43 ++++++++++++++++ lib/nokogiri/xml/reader.rb | 7 +-- test/xml/test_reader.rb | 56 +++++++++++++++++---- 5 files changed, 101 insertions(+), 17 deletions(-) diff --git a/ext/java/nokogiri/XmlReader.java b/ext/java/nokogiri/XmlReader.java index f8b01d550a..615eb85d3e 100644 --- a/ext/java/nokogiri/XmlReader.java +++ b/ext/java/nokogiri/XmlReader.java @@ -144,6 +144,13 @@ public class XmlReader extends RubyObject return currentNode().getAttributesNodes(); } + @JRubyMethod + public IRubyObject + attribute_hash(ThreadContext context) + { + return currentNode().getAttributes(context); + } + @JRubyMethod(name = "attributes?") public IRubyObject attributes_p(ThreadContext context) diff --git a/ext/java/nokogiri/internals/ReaderNode.java b/ext/java/nokogiri/internals/ReaderNode.java index f671075db9..4dc17f62e4 100644 --- a/ext/java/nokogiri/internals/ReaderNode.java +++ b/ext/java/nokogiri/internals/ReaderNode.java @@ -112,9 +112,10 @@ public abstract class ReaderNode getAttributes(ThreadContext context) { final Ruby runtime = context.runtime; - if (attributeList == null) { return runtime.getNil(); } RubyHash hash = RubyHash.newHash(runtime); + if (attributeList == null) { return hash; } for (int i = 0; i < attributeList.length; i++) { + if (isNamespace(attributeList.names.get(i))) { continue; } IRubyObject k = stringOrBlank(runtime, attributeList.names.get(i)); IRubyObject v = stringOrBlank(runtime, attributeList.values.get(i)); hash.fastASetCheckString(runtime, k, v); // hash.op_aset(context, k, v) @@ -150,8 +151,8 @@ public abstract class ReaderNode getNamespaces(ThreadContext context) { final Ruby runtime = context.runtime; - if (namespaces == null) { return runtime.getNil(); } RubyHash hash = RubyHash.newHash(runtime); + if (namespaces == null) { return hash; } for (Map.Entry entry : namespaces.entrySet()) { IRubyObject k = stringOrBlank(runtime, entry.getKey()); IRubyObject v = stringOrBlank(runtime, entry.getValue()); diff --git a/ext/nokogiri/xml_reader.c b/ext/nokogiri/xml_reader.c index 4f87e18f14..81f4bdc275 100644 --- a/ext/nokogiri/xml_reader.c +++ b/ext/nokogiri/xml_reader.c @@ -31,6 +31,7 @@ has_attributes(xmlTextReaderPtr reader) return (0); } +// TODO: merge this function into the `namespaces` method implementation static void Nokogiri_xml_node_namespaces(xmlNodePtr node, VALUE attr_hash) { @@ -181,6 +182,47 @@ rb_xml_reader_attribute_nodes(VALUE rb_reader) return attr_nodes; } +/* + :call-seq: attribute_hash() → Hash + + Get the attributes of the current node as a Hash of names and values. + + See related: #attributes and #namespaces + */ +static VALUE +rb_xml_reader_attribute_hash(VALUE rb_reader) +{ + VALUE rb_attributes = rb_hash_new(); + xmlTextReaderPtr c_reader; + xmlNodePtr c_node; + xmlAttrPtr c_property; + + Data_Get_Struct(rb_reader, xmlTextReader, c_reader); + + if (!has_attributes(c_reader)) { + return rb_attributes; + } + + c_node = xmlTextReaderExpand(c_reader); + c_property = c_node->properties; + while (c_property != NULL) { + VALUE rb_name = NOKOGIRI_STR_NEW2(c_property->name); + VALUE rb_value = Qnil; + xmlChar *c_value = xmlNodeGetContent((xmlNode *)c_property); + + if (c_value) { + rb_value = NOKOGIRI_STR_NEW2(c_value); + xmlFree(c_value); + } + + rb_hash_aset(rb_attributes, rb_name, rb_value); + + c_property = c_property->next; + } + + return rb_attributes; +} + /* * call-seq: * attribute_at(index) @@ -696,6 +738,7 @@ noko_init_xml_reader() rb_define_method(cNokogiriXmlReader, "attribute_at", attribute_at, 1); rb_define_method(cNokogiriXmlReader, "attribute_count", attribute_count, 0); rb_define_method(cNokogiriXmlReader, "attribute_nodes", rb_xml_reader_attribute_nodes, 0); + rb_define_method(cNokogiriXmlReader, "attribute_hash", rb_xml_reader_attribute_hash, 0); rb_define_method(cNokogiriXmlReader, "attributes?", attributes_eh, 0); rb_define_method(cNokogiriXmlReader, "base_uri", rb_xml_reader_base_uri, 0); rb_define_method(cNokogiriXmlReader, "default?", default_eh, 0); diff --git a/lib/nokogiri/xml/reader.rb b/lib/nokogiri/xml/reader.rb index df13216afe..1a00796359 100644 --- a/lib/nokogiri/xml/reader.rb +++ b/lib/nokogiri/xml/reader.rb @@ -87,12 +87,7 @@ def initialize(source, url = nil, encoding = nil) # :nodoc: # # [Returns] (Hash) Attribute names and values def attributes - attrs_hash = attribute_nodes.each_with_object({}) do |node, hash| - hash[node.name] = node.to_s - end - ns = namespaces - attrs_hash.merge!(ns) if ns - attrs_hash + attribute_hash.merge(namespaces) end ### diff --git a/test/xml/test_reader.rb b/test/xml/test_reader.rb index b3019691e6..5b7e888203 100644 --- a/test/xml/test_reader.rb +++ b/test/xml/test_reader.rb @@ -228,23 +228,61 @@ def test_attributes? reader.map(&:attributes?)) end - def test_attributes - reader = Nokogiri::XML::Reader.from_memory(<<-eoxml) - - snuggles! - - eoxml + def test_reader_attributes + reader = Nokogiri::XML::Reader.from_memory(<<~XML) + + snuggles! + + XML assert_empty(reader.attributes) assert_equal([{ "xmlns:tenderlove" => "http://tenderlovemaking.com/", "xmlns" => "http://mothership.connection.com/", }, - {}, { "awesome" => "true" }, {}, { "awesome" => "true" }, {}, + {}, + { "awesome" => "true" }, + {}, + { "awesome" => "true" }, + {}, { "xmlns:tenderlove" => "http://tenderlovemaking.com/", "xmlns" => "http://mothership.connection.com/", },], reader.map(&:attributes)) end + def test_reader_attributes_hash + reader = Nokogiri::XML::Reader.from_memory(<<~XML) + + snuggles! + + XML + assert_empty(reader.attribute_hash) + assert_equal([{}, + {}, + { "awesome" => "true" }, + {}, + { "awesome" => "true" }, + {}, + {},], + reader.map(&:attribute_hash)) + end + + def test_reader_namespaces + reader = Nokogiri::XML::Reader.from_memory(<<~XML) + + snuggles! + + XML + assert_empty(reader.namespaces) + assert_equal([{ "xmlns:tenderlove" => "http://tenderlovemaking.com/", + "xmlns" => "http://mothership.connection.com/", }, + {}, + {}, + {}, + {}, + {}, + { "xmlns:tenderlove" => "http://tenderlovemaking.com/", + "xmlns" => "http://mothership.connection.com/", },], + reader.map(&:namespaces)) + end + def test_attribute_roundtrip reader = Nokogiri::XML::Reader.from_memory(<<-eoxml)