Sort nodeset on demand

Author: tompng

lib/rexml/functions.rb

@@ -84,7 +84,7 @@ def Functions::get_namespace( node_set = nil )
       else
         if node_set.respond_to? :each
           result = []
-          node_set.each do |node|
+          XPathParser.sort(node_set.to_a).each do |node|
             result << yield(node) if node.respond_to?(:namespace)
           end
           result
@@ -146,7 +146,7 @@ def Functions::string( object=@@context[:node] )
       else
         case object
         when Array
-          string(object[0])
+          string(XPathParser.sort(object)[0])
         when Float
           if object.nan?
             "NaN"

lib/rexml/xpath_parser.rb

@@ -155,7 +155,7 @@ def match(path_stack, node)
       result = expr(path_stack, nodeset)
       case result
       when Array # nodeset
-        result.uniq
+        XPathParser.sort(result)
       else
         [result]
       end
@@ -198,7 +198,7 @@ def expr( path_stack, nodeset, context=nil )
           nodeset = [nodeset.first.root_node]
         when :self
           nodeset = step(path_stack) do
-            [nodeset]
+            [XPathParser.sort(nodeset)]
           end
         when :child
           nodeset = step(path_stack) do
@@ -234,7 +234,7 @@ def expr( path_stack, nodeset, context=nil )
             nodesets
           end
         when :ancestor
-          nodeset = step(path_stack, axis_order: :reverse) do
+          nodeset = step(path_stack) do
             nodesets = []
             # new_nodes = {}
             nodeset.each do |node|
@@ -250,7 +250,7 @@ def expr( path_stack, nodeset, context=nil )
             nodesets
           end
         when :ancestor_or_self
-          nodeset = step(path_stack, axis_order: :reverse) do
+          nodeset = step(path_stack) do
             nodesets = []
             # new_nodes = {}
             nodeset.each do |node|
@@ -288,7 +288,7 @@ def expr( path_stack, nodeset, context=nil )
             end.compact
           end
         when :preceding_sibling
-          nodeset = step(path_stack, axis_order: :reverse) do
+          nodeset = step(path_stack) do
             nodeset.map do |node|
               next unless node.respond_to?(:parent)
               next if node.parent.nil?
@@ -300,7 +300,7 @@ def expr( path_stack, nodeset, context=nil )
             end.compact
           end
         when :preceding
-          nodeset = step(path_stack, axis_order: :reverse) do
+          nodeset = step(path_stack) do
             nodeset.map do |node|
               preceding(node)
             end
@@ -393,7 +393,7 @@ def expr( path_stack, nodeset, context=nil )
             # If result is a nodeset, apply following predicates
             path_stack.unshift(:node)
             nodeset = step(path_stack) do
-              [result]
+              [XPathParser.sort(result)]
             end
           else
             return result
@@ -407,7 +407,7 @@ def expr( path_stack, nodeset, context=nil )
       leave(:expr, path_stack, nodeset) if @debug
     end
 
-    def step(path_stack, any_type: :element, axis_order: :forward)
+    def step(path_stack, any_type: :element)
       nodesets = yield
       begin
         enter(:step, path_stack, nodesets) if @debug
@@ -417,18 +417,14 @@ def step(path_stack, any_type: :element, axis_order: :forward)
           predicate_expression = path_stack.shift.dclone
           nodesets = evaluate_predicate(predicate_expression, nodesets)
         end
-        if nodesets.size == 1
-          new_nodeset = axis_order == :forward ? nodesets.first : nodesets.first.reverse
-        else
-          nodes = Set.new.compare_by_identity
-          nodesets.each do |nodeset|
-            nodeset.each do |node|
-              nodes << node
-            end
+
+        nodes = Set.new.compare_by_identity
+        nodesets.each do |nodeset|
+          nodeset.each do |node|
+            nodes << node
           end
-          new_nodeset = sort(nodes.to_a)
         end
-        new_nodeset
+        new_nodeset = nodes.to_a
       ensure
         leave(:step, path_stack, new_nodeset) if @debug
       end
@@ -579,7 +575,9 @@ def leave(tag, *args)
     # in and out of function calls.  If I knew what the index of the nodes was,
     # I wouldn't have to do this.  Maybe add a document IDX for each node?
     # Problems with mutable documents.  Or, rewrite everything.
-    def sort(array_of_nodes)
+    def self.sort(array_of_nodes)
+      return array_of_nodes if array_of_nodes.size <= 1
+
       new_arry = []
       array_of_nodes.each { |node|
         node_idx = []

test/test_jaxen.rb

@@ -87,6 +87,13 @@ def process_value_of(context, variables, namespaces, value_of)
       xpath = value_of.attributes["select"]
       matched = XPath.match(context, xpath, namespaces, variables, strict: true)
 
+      # XPath.match can be a nodeset or a primitive value wrapped in an array.
+      # We need to unwrap primitive value because Functions doesn't accept array which is not a nodeset.
+      unless matched.all? { |node| node.is_a?(REXML::Node) }
+        raise "[BUG] Primitive value should be a single value: #{matched.inspect}" if matched.size != 1
+        matched = matched.first
+      end
+
       message = user_message(context, xpath, matched)
       assert_equal(expected || "",
                    REXML::Functions.string(matched),