Change namespace cache strategy

lib/rexml/doctype.rb

@@ -113,27 +113,26 @@ def node_type
     end
 
     def attributes_of element
-      attribute_declarations_of(element).map do |key, value|
+      raw_attributes = _attlist_mappings[element] || {}
+      raw_attributes.map do |key, value|
         Attribute.new(key, value)
       end
     end
 
     def attribute_of element, attribute
-      attribute_declarations_of(element)[attribute]
+      _attlist_mappings[element]&.[](attribute)
     end
 
-    def attribute_declarations_of(name)
-      raw_attributes = {}
-      decls = select do |child|
-        child.kind_of?(AttlistDecl) && child.element_name == name
-      end
-      decls.each do |child|
+    def _attlist_mappings # :nodoc:
+      mapping = {}
+      grep(AttlistDecl).each do |child|
+        raw_attributes = mapping[child.element_name] ||= {}
         child.each do |key, val|
           # First declaration wins
           raw_attributes[key] = val unless raw_attributes.key? key
         end
       end
-      raw_attributes
+      mapping
     end
 
     def clone

lib/rexml/document.rb

@@ -449,19 +449,6 @@ def document
 
     private
 
-    attr_accessor :namespaces_cache
-
-    # New document level cache is created and available in this block.
-    # This API is thread unsafe. Users can't change this document in this block.
-    def enable_cache
-      @namespaces_cache = {}
-      begin
-        yield
-      ensure
-        @namespaces_cache = nil
-      end
-    end
-
     def build( source )
       Parsers::TreeParser.new( source, self ).parse
     end

lib/rexml/element.rb

@@ -588,12 +588,7 @@ def prefixes
     #   d.elements['//c'].namespaces # => {"x"=>"1", "y"=>"2", "z"=>"3"}
     #
     def namespaces
-      namespaces_cache = document&.__send__(:namespaces_cache)
-      if namespaces_cache
-        namespaces_cache[self] ||= calculate_namespaces
-      else
-        calculate_namespaces
-      end
+      _calculate_namespaces
     end
 
     # :call-seq:
@@ -618,13 +613,10 @@ def namespaces
     #
     def namespace(prefix=nil)
       if prefix.nil?
-        prefix = prefix()
+        _namespace_internal
+      else
+        _namespace_lookup_internal(prefix)
       end
-      prefix = (prefix == '') ? 'xmlns' : prefix.delete_prefix("xmlns:")
-      ns = namespaces[prefix]
-
-      ns = '' if ns.nil? and prefix == 'xmlns'
-      ns
     end
 
     # :call-seq:
@@ -1507,15 +1499,32 @@ def write(output=$stdout, indent=-1, transitive=false, ie_hack=false)
       formatter.write( self, output )
     end
 
-    private
-    def calculate_namespaces
-      if parent
-        parent.namespaces.merge(attributes.namespaces)
-      else
-        attributes.namespaces
-      end
+    # Returns namespace of the element
+    def _namespace_internal(namespaces = _calculate_namespaces) # :nodoc:
+      _namespace_lookup_internal(prefix, namespaces)
+    end
+
+    # Lookup namespace for the given prefix in the context of the element
+    def _namespace_lookup_internal(prefix, namespaces = _calculate_namespaces) # :nodoc:
+      prefix = (prefix == '') ? 'xmlns' : prefix.delete_prefix("xmlns:")
+      ns = namespaces[prefix]
+      ns = '' if ns.nil? and prefix == 'xmlns'
+      ns
+    end
+
+    def _calculate_namespaces(cache_hash = nil, attlist_mappings = nil) # :nodoc:
+      return cache_hash[self] if cache_hash && cache_hash.key?(self)
+
+      attlist_mappings ||= document&.doctype&._attlist_mappings || {}
+      inherited_namespaces = parent ? parent._calculate_namespaces(cache_hash, attlist_mappings) : {}
+      attr_namespaces = attributes._calculate_namespaces(attlist_mappings)
+      namespaces = attr_namespaces.any? ? inherited_namespaces.merge(attr_namespaces) : inherited_namespaces
+      cache_hash[self] = namespaces if cache_hash
+      namespaces
     end
 
+    private
+
     def __to_xpath_helper node
       rv = node.expanded_name.clone
       if node.parent
@@ -2297,7 +2306,7 @@ def each
     #   attrs.get_attribute('nosuch')        # => nil
     #
     def get_attribute( name )
-      fetch(name, nil) || attlist_attributes&.[](name)
+      fetch(name, nil) || attlist_attributes[name]
     end
 
     # :call-seq:
@@ -2372,11 +2381,7 @@ def prefixes
     #   d.root.attributes.namespaces # => {"xmlns"=>"foo", "x"=>"bar", "y"=>"twee"}
     #
     def namespaces
-      namespaces = {}
-      each_effective_attribute do |attribute|
-        namespaces[attribute.name] = attribute.value if attribute.namespace_declaration?
-      end
-      namespaces
+      _calculate_namespaces
     end
 
     # :call-seq:
@@ -2507,33 +2512,41 @@ def get_attribute_ns(namespace, name)
       end
     end
 
+    def _calculate_namespaces(attlist_mappings = nil) # :nodoc:
+      namespaces = {}
+      each_effective_attribute(attlist_mappings) do |attribute|
+        namespaces[attribute.name] = attribute.value if attribute.namespace_declaration?
+      end
+      namespaces
+    end
+
     private
 
     # Iterates over the attributes of the element and those in the DTD.
     # If the element has an attribute with the same name as one in the DTD,
     # the element's attribute takes precedence.
-    def each_effective_attribute
-      return to_enum(__method__) unless block_given?
-      attr = attlist_attributes
-      attr = attr ? attr.merge(self) : self
-      attr.each_value do |attribute|
+    def each_effective_attribute(attlist_mappings = nil)
+      return to_enum(__method__, attlist_mappings) unless block_given?
+      attlist = attlist_attributes(attlist_mappings)
+      attributes = attlist.empty? ? self : attlist.merge(self)
+      attributes.each_value do |attribute|
         yield attribute
       end
     end
 
     alias each_own_attribute each_value
     private :each_own_attribute
 
-    def attlist_attributes
-      doctype = @element.document&.doctype
-      if doctype
-        expn = @element.is_a?(Document) ? doctype.name : @element.expanded_name
-        doctype.attribute_declarations_of(expn).map do |key, value|
-          attr = Attribute.new(key, value)
-          attr.element = @element
-          [key, attr]
-        end.to_h
-      end
+    def attlist_attributes(attlist_mappings = nil)
+      attlist_mappings ||= @element.document&.doctype&._attlist_mappings
+      return {} unless attlist_mappings
+
+      expn = @element.is_a?(Document) ? @element.doctype&.name : @element.expanded_name
+      attlist_mappings[expn]&.map do |key, value|
+        attr = Attribute.new(key, value)
+        attr.element = @element
+        [key, attr]
+      end.to_h
     end
   end
 end

lib/rexml/xpath_parser.rb

@@ -63,6 +63,9 @@ def initialize(strict: false)
       @namespaces = nil
       @variables = {}
       @functions = FunctionsClass.new
+      @attlist_mappings = nil
+      @document = nil
+      @element_namespaces_cache = {}
       @nest = 0
       @strict = strict
     end
@@ -85,18 +88,12 @@ def parse path, node
         node = node.first
       end
 
-      document = node.document
-      if document
-        document.__send__(:enable_cache) do
-          match( path_stack, node )
-        end
-      else
-        match( path_stack, node )
-      end
+      match(path_stack, node)
     end
 
     def get_first path, node
       path_stack = @parser.parse( path )
+      @document = node.document
       first( path_stack, node )
     end
 
@@ -152,6 +149,7 @@ def first( path_stack, node )
 
 
     def match(path_stack, node)
+      @document = node.document
       nodeset = [node]
       result = expr(path_stack, nodeset)
       case result
@@ -167,20 +165,45 @@ def strict?
       @strict
     end
 
-    # Returns a String namespace for a node, given a prefix
+    # Returns a String namespace for a prefix used in xpath
     # The rules are:
     #
     #  1. Use the supplied namespace mapping first.
-    #  2. If no mapping was supplied, use the context node to look up the namespace
-    def get_namespace( node, prefix )
+    #  2. If no mapping was supplied, use the context node to look up the namespace as a fallback.
+    def get_xpath_namespace(node, prefix)
       if @namespaces
         @namespaces[prefix] || ''
+      elsif node.node_type == :element
+        element_namespace_lookup(node, prefix)
       else
-        return node.namespace( prefix ) if node.node_type == :element
         ''
       end
     end
 
+    # Returns attribute's namespace URI while caching the
+    # intermediate result to speed up retrieval of namespaces
+    def attribute_namespace(attribute)
+      attribute.prefix == '' ? '' : element_namespace_lookup(attribute.element, attribute.prefix)
+    end
+
+    # Return element's namespace URI while caching the
+    # intermediate result to speed up retrieval of namespaces
+    def element_namespace(element)
+      element._namespace_internal(element_namespaces(element))
+    end
+
+    # Returns a hash of namespaces for the given element while caching the
+    # intermediate result to speed up retrieval of namespaces
+    def element_namespaces(element)
+      @attlist_mappings ||= @document&.doctype&._attlist_mappings || {}
+      element._calculate_namespaces(@element_namespaces_cache, @attlist_mappings)
+    end
+
+    # Returns namespace of the prefix in the context of the element,
+    # while caching the intermediate result to speed up retrieval of namespaces
+    def element_namespace_lookup(element, prefix)
+      element._namespace_lookup_internal(prefix, element_namespaces(element))
+    end
 
     # Expr takes a stack of path elements and a set of nodes (either a Parent
     # or an Array and returns an Array of matching nodes
@@ -642,20 +665,20 @@ def node_test(path_stack, any_type: :element)
               node.name == name
             elsif prefix.empty?
               if strict?
-                node.name == name and node.namespace == ""
+                node.name == name and element_namespace(node) == ""
               else
-                node.name == name and node.namespace == get_namespace(node, prefix)
+                node.name == name and element_namespace(node) == get_xpath_namespace(node, prefix)
               end
             else
-              node.name == name and node.namespace == get_namespace(node, prefix)
+              node.name == name and element_namespace(node) == get_xpath_namespace(node, prefix)
             end
           when :attribute
             if prefix.nil?
               node.name == name
             elsif prefix.empty?
-              node.name == name and node.namespace == ""
+              node.name == name and attribute_namespace(node) == ""
             else
-              node.name == name and node.namespace == get_namespace(node.element, prefix)
+              node.name == name and attribute_namespace(node) == get_xpath_namespace(node.element, prefix)
             end
           else
             false
@@ -666,11 +689,11 @@ def node_test(path_stack, any_type: :element)
         ->(node) do
           case node.node_type
           when :element
-            namespaces = @namespaces || node.namespaces
-            node.namespace == namespaces[prefix]
+            namespaces = @namespaces || element_namespaces(node)
+            element_namespace(node) == namespaces[prefix]
           when :attribute
-            namespaces = @namespaces || node.element.namespaces
-            node.namespace == namespaces[prefix]
+            namespaces = @namespaces || element_namespaces(node.element)
+            attribute_namespace(node) == namespaces[prefix]
           else
             false
           end

test/test_namespace.rb

@@ -1,7 +1,9 @@
 # frozen_string_literal: false
+require "core_assertions"
 
 module REXMLTests
   class TestNamespace < Test::Unit::TestCase
+    include Test::Unit::CoreAssertions
     include Helper::Fixture
     include REXML
 
@@ -34,5 +36,25 @@ def test_xml_namespace
       assert_equal("http://www.w3.org/XML/1998/namespace",
                    document.root.namespace("xml"))
     end
+
+    def test_deep_element_namespace_linear
+      omit('Recursion too deep on JRuby') if RUBY_ENGINE == "jruby"
+
+      max_depth = 3000
+      xml = <<~XML
+        <root xmlns="one">#{'<a>' * max_depth + '</a>' * max_depth}</root>
+      XML
+      doc = Document.new(xml)
+      prepare_element = ->(depth) do
+        node = doc.root
+        depth.times { node = node.first }
+        node || raise
+      end
+
+      assert_linear_performance([30, 100, 300, 1000, 3000], rehearsal: 10) do |depth|
+        elem = prepare_element.call(depth)
+        elem.namespace
+      end
+    end
   end
 end

test/xpath/test_base.rb

@@ -1320,16 +1320,6 @@ def test_namespaces_0
       assert_equal( 1,  XPath.match( d, "//x:*" ).size )
     end
 
-    def test_namespaces_cache
-      doc = Document.new("<a xmlns='1'><b/></a>")
-      assert_equal("<b/>", XPath.first(doc, "//b[namespace-uri()='1']").to_s)
-      assert_nil(XPath.first(doc, "//b[namespace-uri()='']"))
-
-      doc.root.delete_namespace
-      assert_nil(XPath.first(doc, "//b[namespace-uri()='1']"))
-      assert_equal("<b/>", XPath.first(doc, "//b[namespace-uri()='']").to_s)
-    end
-
     def test_ticket_71
       doc = Document.new(%Q{<root xmlns:ns1="xyz" xmlns:ns2="123"><element ns1:attrname="foo" ns2:attrname="bar"/></root>})
       el = doc.root.elements[1]