Skip to content

Commit

Permalink
Reworking namespace cahce to deal with issues sparklemotion#943 and s…
Browse files Browse the repository at this point in the history
…parklemotion#940.  This allows for namespace prfixes to the redeclared at an element level, allows for default namespaces to be redeclared at an element level and fixes issues related to adding namepsaces delcarations on the document for use in xpath calls
  • Loading branch information
rdingwell committed Aug 23, 2013
1 parent 779b318 commit 4888e1d
Show file tree
Hide file tree
Showing 11 changed files with 89 additions and 112 deletions.
2 changes: 1 addition & 1 deletion Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ HOE = Hoe.spec 'nokogiri' do

self.extra_rdoc_files = FileList['*.rdoc','ext/nokogiri/*.c']

self.licenses = ['MIT']
# self.licenses = ['MIT']

This comment has been minimized.

Copy link
@leejarvis

leejarvis Sep 26, 2013

Why is this being commented out?


self.clean_globs += [
'nokogiri.gemspec',
Expand Down
4 changes: 2 additions & 2 deletions ext/java/nokogiri/XmlDocument.java
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ private void createAndCacheNamespaces(Ruby ruby, Node node) {
public XmlDocument(Ruby ruby, RubyClass klass, Document document, XmlDocument contextDoc) {
super(ruby, klass, document);
nsCache = contextDoc.getNamespaceCache();
XmlNamespace default_ns = nsCache.getDefault();
XmlNamespace default_ns = nsCache.getDefault(document.getDocumentElement());
String default_href = rubyStringToString(default_ns.href(ruby.getCurrentContext()));
resolveNamespaceIfNecessary(ruby.getCurrentContext(), document.getDocumentElement(), default_href);
}
Expand All @@ -181,7 +181,7 @@ private void resolveNamespaceIfNecessary(ThreadContext context, Node node, Strin
if (nodePrefix == null) { // default namespace
NokogiriHelpers.renameNode(node, default_href, node.getNodeName());
} else {
XmlNamespace xmlNamespace = nsCache.get(nodePrefix);
XmlNamespace xmlNamespace = nsCache.get(nodePrefix, node);
String href = rubyStringToString(xmlNamespace.href(context));
NokogiriHelpers.renameNode(node, href, node.getNodeName());
}
Expand Down
13 changes: 12 additions & 1 deletion ext/java/nokogiri/XmlDocumentFragment.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import static nokogiri.internals.NokogiriHelpers.rubyStringToString;

import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.regex.Matcher;
Expand Down Expand Up @@ -102,7 +103,17 @@ public static IRubyObject rbNew(ThreadContext context, IRubyObject cls, IRubyObj

//TODO: Get namespace definitions from doc.
if (args.length == 3 && args[2] != null && args[2] instanceof XmlElement) {
fragment.fragmentContext = (XmlElement)args[2];
XmlElement elm = (XmlElement)args[2];

RubyArray namespaces = (RubyArray)elm.namespace_definitions(context);
for (Object obj : namespaces) {
XmlNamespace namespace=(XmlNamespace)obj;
doc.getNamespaceCache().put(namespace,fragment.node);
//XmlNamespace ns = XmlNamespace.createFromPrefixAndHref(fragment.node, namespace.prefix(context), namespace.href(context));
}

fragment.fragmentContext = (XmlElement)args[2];

}
RuntimeHelpers.invoke(context, fragment, "initialize", args);
return fragment;
Expand Down
10 changes: 6 additions & 4 deletions ext/java/nokogiri/XmlNamespace.java
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public static XmlNamespace createFromAttr(Ruby runtime, Attr attr) {
// check namespace cache
XmlDocument xmlDocument = (XmlDocument)getCachedNodeOrCreate(runtime, attr.getOwnerDocument());
xmlDocument.initializeNamespaceCacheIfNecessary();
XmlNamespace xmlNamespace = xmlDocument.getNamespaceCache().get(prefixValue, hrefValue);
XmlNamespace xmlNamespace = xmlDocument.getNamespaceCache().get(prefixValue, attr.getOwnerElement());
if (xmlNamespace != null) return xmlNamespace;

// creating XmlNamespace instance
Expand All @@ -136,7 +136,7 @@ public static XmlNamespace createFromPrefixAndHref(Node owner, IRubyObject prefi
// check namespace cache
XmlDocument xmlDocument = (XmlDocument)getCachedNodeOrCreate(runtime, document);
xmlDocument.initializeNamespaceCacheIfNecessary();
XmlNamespace xmlNamespace = xmlDocument.getNamespaceCache().get(prefixValue, hrefValue);
XmlNamespace xmlNamespace = xmlDocument.getNamespaceCache().get(prefixValue, owner);
if (xmlNamespace != null) return xmlNamespace;

// creating XmlNamespace instance
Expand All @@ -148,7 +148,7 @@ public static XmlNamespace createFromPrefixAndHref(Node owner, IRubyObject prefi
}
Attr attrNode = document.createAttribute(attrName);
attrNode.setNodeValue(hrefValue);

This comment has been minimized.

Copy link
@leejarvis

leejarvis Sep 26, 2013

Could you squash these whitespace changes?

// initialize XmlNamespace object
namespace.init(attrNode, prefix, href, prefixValue, hrefValue, xmlDocument);

Expand All @@ -164,7 +164,9 @@ public static XmlNamespace createDefaultNamespace(Ruby runtime, Node owner) {
Document document = owner.getOwnerDocument();
// check namespace cache
XmlDocument xmlDocument = (XmlDocument)getCachedNodeOrCreate(runtime, document);
XmlNamespace xmlNamespace = xmlDocument.getNamespaceCache().get(prefixValue, hrefValue);

//
XmlNamespace xmlNamespace = xmlDocument.getNamespaceCache().getFromHierarchy(prefixValue, owner);
if (xmlNamespace != null) return xmlNamespace;

// creating XmlNamespace instance
Expand Down
9 changes: 6 additions & 3 deletions ext/java/nokogiri/XmlNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -589,9 +589,10 @@ public IRubyObject add_namespace_definition(ThreadContext context,
}
else if (node.getNodeType() == Node.ATTRIBUTE_NODE) namespaceOwner = ((Attr)node).getOwnerElement();
else namespaceOwner = node.getParentNode();

XmlNamespace ns = XmlNamespace.createFromPrefixAndHref(namespaceOwner, prefix, href);
if (node != namespaceOwner) {
this.node = NokogiriHelpers.renameNode(node, ns.getHref(), ns.getPrefix() + node.getLocalName());
this.node = NokogiriHelpers.renameNode(node, ns.getHref(), ns.getPrefix() + ":" + node.getLocalName());
}

updateNodeNamespaceIfNecessary(context, ns);
Expand Down Expand Up @@ -1038,7 +1039,7 @@ public IRubyObject namespace(ThreadContext context) {
XmlDocument xmlDocument = (XmlDocument) doc;
NokogiriNamespaceCache nsCache = xmlDocument.getNamespaceCache();
String prefix = node.getPrefix();
XmlNamespace namespace = nsCache.get(prefix == null ? "" : prefix, node.getNamespaceURI());
XmlNamespace namespace = nsCache.getFromHierarchy(prefix == null ? "" : prefix, node);
if (namespace == null || namespace.isEmpty()) {
return context.getRuntime().getNil();
}
Expand Down Expand Up @@ -1284,7 +1285,7 @@ public IRubyObject set_namespace(ThreadContext context, IRubyObject namespace) {
Node n = node;
String prefix = n.getPrefix();
String href = n.getNamespaceURI();
((XmlDocument)doc).getNamespaceCache().remove(prefix == null ? "" : prefix, href);
((XmlDocument)doc).getNamespaceCache().remove(prefix == null ? "" : prefix, node);
this.node = NokogiriHelpers.renameNode(n, null, NokogiriHelpers.getLocalPart(n.getNodeName()));
}
} else {
Expand All @@ -1302,6 +1303,8 @@ public IRubyObject set_namespace(ThreadContext context, IRubyObject namespace) {
// and keep the return value. -mbklein
String new_name = NokogiriHelpers.newQName(prefix, node);
this.node = NokogiriHelpers.renameNode(node, href, new_name);
((XmlDocument)doc).getNamespaceCache().put(ns, node);

}

return this;
Expand Down
4 changes: 2 additions & 2 deletions ext/java/nokogiri/internals/NokogiriHelpers.java
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public static IRubyObject getCachedNodeOrCreate(Ruby ruby, Node node) {
String prefix = getLocalNameForNamespace(((Attr)node).getName());
prefix = prefix != null ? prefix : "";
String href = ((Attr)node).getValue();
XmlNamespace xmlNamespace = xmlDocument.getNamespaceCache().get(prefix, href);
XmlNamespace xmlNamespace = xmlDocument.getNamespaceCache().get(prefix, node);
if (xmlNamespace != null) return xmlNamespace;
else return XmlNamespace.createFromAttr(ruby, (Attr)node);
}
Expand Down Expand Up @@ -637,7 +637,7 @@ public static String canonicalizeWhitespce(String s) {

public static String newQName(String newPrefix, Node node) {
String tagName = getLocalPart(node.getNodeName());
if(newPrefix == null) {
if(newPrefix == null || newPrefix.equals("")) {
return tagName;
} else {
return newPrefix + ":" + tagName;
Expand Down
143 changes: 49 additions & 94 deletions ext/java/nokogiri/internals/NokogiriNamespaceCache.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@

package nokogiri.internals;

import static nokogiri.internals.NokogiriHelpers.isNamespace;

import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
Expand All @@ -42,7 +40,6 @@
import nokogiri.XmlNamespace;

import org.w3c.dom.Attr;
import org.w3c.dom.NamedNodeMap;
import org.w3c.dom.Node;

/**
Expand All @@ -53,128 +50,86 @@
*/
public class NokogiriNamespaceCache {

private List<Long> keys; // order matters.
private Map<Integer, CacheEntry> cache; // pair of the index of a given key and entry

private Map<Node, Map<String, XmlNamespace>> cache; // pair of the index of a given key and entry
private XmlNamespace defaultNamespace = null;

public NokogiriNamespaceCache() {
keys = new ArrayList<Long>();
cache = new LinkedHashMap<Integer, CacheEntry>();
cache = new LinkedHashMap<Node, Map<String, XmlNamespace>>();
}

private Long hashCode(String prefix, String href) {
long prefix_hash = prefix.hashCode();
long href_hash = href.hashCode();
return prefix_hash << 31 | href_hash;
private Map<String, XmlNamespace> getNodeCache(Node node){
Map<String, XmlNamespace> nodeMap = cache.get(node);
if(nodeMap == null){
nodeMap = new LinkedHashMap<String, XmlNamespace>();
cache.put(node, nodeMap);
}
return nodeMap;
}

public XmlNamespace get(String prefix, String href) {
public XmlNamespace get(String prefix, Node node) {
// prefix should not be null.
// In case of a default namespace, an empty string should be given to prefix argument.
if (prefix == null || href == null) return null;
Long hash = hashCode(prefix, href);
Integer index = keys.indexOf(hash);
if (index != -1) return cache.get(index).namespace;
return null;

return getNodeCache(node).get(prefix);
}

public XmlNamespace getDefault() {
return defaultNamespace;

public XmlNamespace getFromHierarchy(String prefix, Node node){
if(node == null) return null;
XmlNamespace namespace = get(prefix,node);
if(namespace == null){
Node owner = node.getParentNode();
if(node instanceof Attr){
owner = ((Attr)node).getOwnerElement();
}
if(owner != node.getOwnerDocument() ){
namespace = getFromHierarchy(prefix,owner);
}
}
return namespace;
}

public XmlNamespace get(String prefix) {
if (prefix == null) return defaultNamespace;
long h = prefix.hashCode();
Long hash = h << 31;
Long mask = 0xFF00L;
for (int i=0; i < keys.size(); i++) {
if ((keys.get(i) & mask) == hash) {
return cache.get(i).namespace;
}
}
return null;
public XmlNamespace getDefault(Node node) {
return getFromHierarchy("", node);
}



public List<XmlNamespace> get(Node node) {
List<XmlNamespace> namespaces = new ArrayList<XmlNamespace>();
for (int i=0; i < keys.size(); i++) {
CacheEntry entry = cache.get(i);
if (entry.isOwner(node)) {
namespaces.add(entry.namespace);
}
}
Map<String, XmlNamespace> nodeMap = getNodeCache(node);
namespaces.addAll( nodeMap.values());
return namespaces;
}

public void put(XmlNamespace namespace, Node ownerNode) {
// prefix should not be null.
// In case of a default namespace, an empty string should be given to prefix argument.
public void put(XmlNamespace namespace, Node node) {
// only add if there is not a prefix mapping already, this is consitent with C Ruby nokogiri
//implementation
XmlNamespace have = getFromHierarchy(namespace.getPrefix(),node);
if(have != null && have.getHref().equals(namespace.getHref())){
return;
}
String prefixString = namespace.getPrefix();
String hrefString = namespace.getHref();
Long hash = hashCode(prefixString, hrefString);
Integer index;
if ((index = keys.indexOf(hash)) != -1) {
return;
} else {
keys.add(hash);
index = keys.size() - 1;
CacheEntry entry = new CacheEntry(namespace, ownerNode);
cache.put(index, entry);
if ("".equals(prefixString)) defaultNamespace = namespace;
Map<String, XmlNamespace> nodeMap = getNodeCache(node);
if(nodeMap.get(prefixString) == null){
nodeMap.put(prefixString, namespace);
}
}

public void remove(String prefix, String href) {
if (prefix == null || href == null) return;
Long hash = hashCode(prefix, href);
Integer index = keys.indexOf(hash);
if (index != -1) {
cache.remove(index);
}
keys.remove(index);
public void remove(String prefix, Node node) {
getNodeCache(node).remove(prefix);
}

public void clear() {
// removes namespace declarations from node
for (int i=0; i < keys.size(); i++) {
CacheEntry entry = cache.get(i);
NamedNodeMap attributes = entry.ownerNode.getAttributes();
for (int j=0; j<attributes.getLength(); j++) {
String name = ((Attr)attributes.item(j)).getName();
if (isNamespace(name)) {
attributes.removeNamedItem(name);
}
}
}
keys.clear();
cache.clear();
defaultNamespace = null;
}

public void replaceNode(Node oldNode, Node newNode) {
for (int i=0; i < keys.size(); i++) {
CacheEntry entry = cache.get(i);
if (entry.isOwner(oldNode)) {
entry.replaceOwner(newNode);
}
}
cache.put(newNode, cache.get(oldNode));
cache.remove(oldNode);
}

private class CacheEntry {
private XmlNamespace namespace;
private Node ownerNode;

CacheEntry(XmlNamespace namespace, Node ownerNode) {
this.namespace = namespace;
this.ownerNode = ownerNode;
}

public Boolean isOwner(Node n) {
return this.ownerNode.isSameNode(n);
}

public void replaceOwner(Node newNode) {
this.ownerNode = newNode;
}
}

}
3 changes: 2 additions & 1 deletion lib/nokogiri/xml/document_fragment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ class DocumentFragment < Nokogiri::XML::Node
# to +ctx+.
def initialize document, tags = nil, ctx = nil
return self unless tags

children = if ctx
# Fix for issue#490
if Nokogiri.jruby?

# fix for issue #770
ctx.parse("<root #{namespace_declarations(ctx)}>#{tags}</root>").children
else
Expand All @@ -23,6 +23,7 @@ def initialize document, tags = nil, ctx = nil
.xpath("/root/node()")
end
children.each { |child| child.parent = self }

end

###
Expand Down
3 changes: 3 additions & 0 deletions test/xml/test_attr.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ def test_parsing_attribute_namespace
end

def test_setting_attribute_namespace

doc = Nokogiri::XML <<-EOXML
<root xmlns='http://google.com/' xmlns:f='http://flavorjon.es/'>
<div f:myattr='foo'></div>
Expand All @@ -57,6 +58,8 @@ def test_setting_attribute_namespace
node = doc.at_css "div"
attr = node.attributes["myattr"]
attr.add_namespace("fizzle", "http://fizzle.com/")

attr.namespace
assert_equal "http://fizzle.com/", attr.namespace.href
end
end
Expand Down
6 changes: 4 additions & 2 deletions test/xml/test_document_fragment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,15 @@ def test_fragment_search
end

def test_fragment_without_a_namespace_does_not_get_a_namespace

doc = Nokogiri::XML <<-EOX
<root xmlns="http://tenderlovemaking.com/" xmlns:foo="http://flavorjon.es/" xmlns:bar="http://google.com/">
<root xmlns:foo="http://flavorjon.es/" xmlns:bar="http://google.com/">
<foo:existing></foo:existing>
</root>
EOX

frag = doc.fragment "<newnode></newnode>"
assert_nil frag.namespace
assert_nil frag.child.namespace
end

def test_fragment_namespace_resolves_against_document_root
Expand Down
Loading

0 comments on commit 4888e1d

Please sign in to comment.