XPath searches now work with compiled expressions

Author: flavorjones

ext/nokogiri/nokogiri.h

@@ -211,6 +211,8 @@ VALUE noko_xml_sax_parser_context_wrap(VALUE klass, xmlParserCtxtPtr c_context);
 xmlParserCtxtPtr noko_xml_sax_parser_context_unwrap(VALUE rb_context);
 void noko_xml_sax_parser_context_set_encoding(xmlParserCtxtPtr c_context, VALUE rb_encoding);
 
+xmlXPathCompExprPtr noko_xml_xpath_expression_unwrap(VALUE rb_expression);
+
 #define DOC_RUBY_OBJECT_TEST(x) ((nokogiriTuplePtr)(x->_private))
 #define DOC_RUBY_OBJECT(x) (((nokogiriTuplePtr)(x->_private))->doc)
 #define DOC_UNLINKED_NODE_HASH(x) (((nokogiriTuplePtr)(x->_private))->unlinkedNodes)

ext/nokogiri/xml_xpath_context.c

@@ -382,6 +382,7 @@ noko_xml_xpath_context_evaluate(int argc, VALUE *argv, VALUE rb_context)
   VALUE rb_expression = Qnil;
   VALUE rb_function_lookup_handler = Qnil;
   xmlChar *c_expression_str = NULL;
+  xmlXPathCompExprPtr c_expression_comp = NULL;
   VALUE rb_errors = rb_ary_new();
   xmlXPathObjectPtr c_xpath_object;
   VALUE rb_xpath_object = Qnil;
@@ -390,7 +391,11 @@ noko_xml_xpath_context_evaluate(int argc, VALUE *argv, VALUE rb_context)
 
   rb_scan_args(argc, argv, "11", &rb_expression, &rb_function_lookup_handler);
 
-  c_expression_str = (xmlChar *)StringValueCStr(rb_expression);
+  if (rb_obj_is_kind_of(rb_expression, cNokogiriXmlXpathExpression)) {
+    c_expression_comp = noko_xml_xpath_expression_unwrap(rb_expression);
+  } else {
+    c_expression_str = (xmlChar *)StringValueCStr(rb_expression);
+  }
 
   if (Qnil != rb_function_lookup_handler) {
     /* FIXME: not sure if this is the correct place to shove private data. */
@@ -405,7 +410,11 @@ noko_xml_xpath_context_evaluate(int argc, VALUE *argv, VALUE rb_context)
   xmlSetStructuredErrorFunc((void *)rb_errors, noko__error_array_pusher);
   xmlSetGenericErrorFunc((void *)rb_errors, generic_exception_pusher);
 
-  c_xpath_object = xmlXPathEvalExpression(c_expression_str, c_context);
+  if (c_expression_comp) {
+    c_xpath_object = xmlXPathCompiledEval(c_expression_comp, c_context);
+  } else {
+    c_xpath_object = xmlXPathEvalExpression(c_expression_str, c_context);
+  }
 
   xmlSetStructuredErrorFunc(NULL, NULL);
   xmlSetGenericErrorFunc(NULL, NULL);

ext/nokogiri/xml_xpath_expression.c

@@ -27,17 +27,32 @@ static const rb_data_type_t _noko_xml_xpath_expression_type = {
 static VALUE
 noko_xml_xpath_expression_s_new(VALUE klass, VALUE rb_input)
 {
-  xmlXPathCompExprPtr c_expr;
+  xmlXPathCompExprPtr c_expr = NULL;
   VALUE rb_expr = Qnil;
+  VALUE rb_errors = rb_ary_new();
+
+  xmlSetStructuredErrorFunc((void *)rb_errors, noko__error_array_pusher);
 
   c_expr = xmlXPathCompile((const xmlChar *)StringValueCStr(rb_input));
-  if (c_expr) {
-    rb_expr = TypedData_Wrap_Struct(klass, &_noko_xml_xpath_expression_type, c_expr);
+
+  xmlSetStructuredErrorFunc(NULL, NULL);
+
+  if (c_expr == NULL) {
+    rb_exc_raise(rb_ary_entry(rb_errors, 0));
   }
 
+  rb_expr = TypedData_Wrap_Struct(klass, &_noko_xml_xpath_expression_type, c_expr);
   return rb_expr;
 }
 
+xmlXPathCompExprPtr
+noko_xml_xpath_expression_unwrap(VALUE rb_expression)
+{
+  xmlXPathCompExprPtr c_expression;
+  TypedData_Get_Struct(rb_expression, xmlXPathCompExpr, &_noko_xml_xpath_expression_type, c_expression);
+  return c_expression;
+}
+
 void
 noko_init_xml_xpath_expression(void)
 {

lib/nokogiri/xml/searchable.rb

@@ -209,7 +209,7 @@ def >(selector) # rubocop:disable Naming/BinaryOperatorParameterName
 
       def extract_params(params) # :nodoc:
         handler = params.find do |param|
-          ![Hash, String, Symbol].include?(param.class)
+          ![Hash, String, Symbol, XPath::Expression].include?(param.class)
         end
         params -= [handler] if handler
 

test/xml/test_xpath.rb

@@ -719,9 +719,9 @@ def collision(nodes)
           node = doc.at_xpath("//ns:child", { "ns" => "http://nokogiri.org/ns1" })
           assert_equal("ns1", node.text)
 
-          assert_raises(XPath::SyntaxError) {
+          assert_raises(XPath::SyntaxError) do
             doc.at_xpath("//ns:child")
-          }
+          end
 
           node = doc.at_xpath("//child")
           assert_nil(node)
@@ -743,9 +743,9 @@ def collision(nodes)
           doc.xpath("//xmlns:child[nokogiri:thing(.)]", @handler)
           assert_equal(1, @handler.things.length)
 
-          assert_raises(XPath::SyntaxError) {
+          assert_raises(XPath::SyntaxError) do
             doc.xpath("//xmlns:child[nokogiri:thing(.)]")
-          }
+          end
 
           doc.xpath("//xmlns:child[nokogiri:thing(.)]", @handler)
           assert_equal(2, @handler.things.length)
@@ -763,34 +763,36 @@ def collision(nodes)
           nodes = @xml.xpath("//address[@domestic=$value]", nil, value: "Yes")
           assert_equal(4, nodes.length)
 
-          assert_raises(XPath::SyntaxError) {
+          assert_raises(XPath::SyntaxError) do
             @xml.xpath("//address[@domestic=$value]")
-          }
+          end
 
           nodes = @xml.xpath("//address[@domestic=$value]", nil, value: "Qwerty")
           assert_empty(nodes)
 
-          assert_raises(XPath::SyntaxError) {
+          assert_raises(XPath::SyntaxError) do
             @xml.xpath("//address[@domestic=$value]")
-          }
+          end
 
           nodes = @xml.xpath("//address[@domestic=$value]", nil, value: "Yes")
           assert_equal(4, nodes.length)
         end
       end
 
       describe "compiled" do
-        let(:doc) {
-          Nokogiri::XML::Document.parse(<<~XML)
+        let(:xml) {
+          <<~XML
             <root xmlns="http://nokogiri.org/default" xmlns:ns1="http://nokogiri.org/ns1">
               <child>default</child>
               <ns1:child>ns1</ns1:child>
             </root>
           XML
         }
 
+        let(:doc) { Nokogiri::XML::Document.parse(xml) }
+
         describe "XPath expressions" do
-          it "works" do
+          it "works in the trivial case" do
             expr = Nokogiri::XML::XPath.expression("//xmlns:child")
 
             result = doc.xpath(expr)
@@ -800,13 +802,55 @@ def collision(nodes)
             end
           end
 
-          it "can be evaluated in different documents"
+          it "works as expected with namespace bindings" do
+            expr = Nokogiri::XML::XPath.expression("//ns:child")
 
-          it "work with function handlers"
+            node = doc.at_xpath(expr, { "ns" => "http://nokogiri.org/ns1" })
+            assert_equal("ns1", node.text)
 
-          it "work with variable bindings"
+            assert_raises(XPath::SyntaxError) do
+              doc.at_xpath("//ns:child")
+            end
+          end
 
-          it "work with namespace bindings"
+          it "works as expected with a function handler" do
+            expr = Nokogiri::XML::XPath.expression("//xmlns:child[nokogiri:thing(.)]")
+
+            doc.xpath(expr, @handler)
+            assert_equal(1, @handler.things.length)
+
+            assert_raises(XPath::SyntaxError) do
+              doc.xpath("//xmlns:child[nokogiri:thing(.)]")
+            end
+          end
+
+          it "works as expected with bound variables" do
+            expr = Nokogiri::XML::XPath.expression("//address[@domestic=$value]")
+
+            nodes = @xml.xpath("//address[@domestic=$value]", nil, value: "Yes")
+            assert_equal(4, nodes.length)
+
+            assert_raises(XPath::SyntaxError) do
+              @xml.xpath(expr)
+            end
+          end
+
+          it "can be evaluated in different documents" do
+            doc1 = Nokogiri::XML::Document.parse(xml)
+            doc2 = Nokogiri::XML::Document.parse(xml)
+
+            expr = Nokogiri::XML::XPath.expression("//xmlns:child")
+
+            result1 = doc1.xpath(expr)
+            result2 = doc2.xpath(expr)
+
+            assert_pattern do
+              result1 => [{name: "child", namespace: { href: "http://nokogiri.org/default" }}]
+            end
+            assert_pattern do
+              result2 => [{name: "child", namespace: { href: "http://nokogiri.org/default" }}]
+            end
+          end
         end
 
         describe "CSS selectors" do

test/xml/test_xpath_expression.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+require "helper"
+
+describe Nokogiri::XML::XPath::Expression do
+  it ".new" do
+    assert_kind_of(Nokogiri::XML::XPath::Expression, Nokogiri::XML::XPath::Expression.new("//foo"))
+  end
+
+  it "raises an exception when there are compile-time errors" do
+    assert_raises(Nokogiri::XML::XPath::SyntaxError) do
+      Nokogiri::XML::XPath.expression("//foo[")
+    end
+  end
+end
+
+describe Nokogiri::XML::XPath do
+  it "XPath.expression" do
+    assert_kind_of(Nokogiri::XML::XPath::Expression, Nokogiri::XML::XPath.expression("//foo"))
+  end
+end