Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

JRuby & Builder namespace improvements: Take 2 #846

Closed
wants to merge 6 commits into from
Closed

JRuby & Builder namespace improvements: Take 2 #846

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
887a1b4
Added thorough namespace compliance testing for parsed, created, and …
mbklein Dec 13, 2012
931f90d
* Fix the NAMESPACE_ERR issues (e.g, Issue #801)
mbklein Dec 11, 2012
dd77602
Allow namespaces to be declared on first use in Builder just as they …
mbklein Dec 18, 2012
bf165c8
Make sure the namespace test knows where to find bacon
mbklein Dec 20, 2012
5d020dd
Port JRuby namespace changes to CRuby extension
mbklein Feb 13, 2013
8bec571
Disambiguate and shore up namespace tests
mbklein Feb 14, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions ext/java/nokogiri/XmlDocument.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,11 @@ private void resolveNamespaceIfNecessary(ThreadContext context, Node node, Strin
if (node == null) return;
String nodePrefix = node.getPrefix();
if (nodePrefix == null) { // default namespace
node.getOwnerDocument().renameNode(node, default_href, node.getNodeName());
NokogiriHelpers.renameNode(node, default_href, node.getNodeName());
} else {
XmlNamespace xmlNamespace = nsCache.get(nodePrefix);
String href = rubyStringToString(xmlNamespace.href(context));
node.getOwnerDocument().renameNode(node, href, node.getNodeName());
NokogiriHelpers.renameNode(node, href, node.getNodeName());
}
resolveNamespaceIfNecessary(context, node.getNextSibling(), default_href);
NodeList children = node.getChildNodes();
Expand Down Expand Up @@ -358,15 +358,15 @@ private void removeNamespceRecursively(ThreadContext context, XmlNode xmlNode) {
Node node = xmlNode.node;
if (node.getNodeType() == Node.ELEMENT_NODE) {
node.setPrefix(null);
node.getOwnerDocument().renameNode(node, null, node.getLocalName());
NokogiriHelpers.renameNode(node, null, node.getLocalName());
NamedNodeMap attrs = node.getAttributes();
for (int i=0; i<attrs.getLength(); i++) {
Attr attr = (Attr) attrs.item(i);
if (isNamespace(attr.getNodeName())) {
((org.w3c.dom.Element)node).removeAttributeNode(attr);
} else {
attr.setPrefix(null);
attr.getOwnerDocument().renameNode(attr, null, attr.getLocalName());
NokogiriHelpers.renameNode(attr, null, attr.getLocalName());
}
}
}
Expand Down
68 changes: 48 additions & 20 deletions ext/java/nokogiri/XmlNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,12 +328,15 @@ protected void init(ThreadContext context, IRubyObject[] args) {

Element element = null;
String node_name = rubyStringToString(name);
try {
element = document.createElementNS(null, node_name);
} catch (org.w3c.dom.DOMException e) {
// issue#683 NAMESPACE_ERR is thrown from RDF::RDFXML::Writer.new
// retry without namespace
String prefix = NokogiriHelpers.getPrefix(node_name);
if (prefix == null) {
element = document.createElement(node_name);
} else {
String namespace_uri = null;
if (document.getDocumentElement() != null) {
namespace_uri = document.getDocumentElement().lookupNamespaceURI(prefix);
}
element = document.createElementNS(namespace_uri, node_name);
}
setNode(context, element);
}
Expand Down Expand Up @@ -451,33 +454,45 @@ public void post_add_child(ThreadContext context, XmlNode current, XmlNode child
public void relink_namespace(ThreadContext context) {
if (node instanceof Element) {
Element e = (Element) node;
String prefix = e.getPrefix();
String currentNS = e.getNamespaceURI();
if (prefix == null && currentNS == null) {
prefix = NokogiriHelpers.getPrefix(e.getNodeName());
} else if (currentNS != null) {
prefix = e.lookupPrefix(currentNS);
}
e.getOwnerDocument().setStrictErrorChecking(false);
e.getOwnerDocument().renameNode(e, e.lookupNamespaceURI(e.getPrefix()), e.getNodeName());
String nsURI = e.lookupNamespaceURI(prefix);
this.node = NokogiriHelpers.renameNode(e, nsURI, e.getNodeName());

if (e.hasAttributes()) {
NamedNodeMap attrs = e.getAttributes();

for (int i = 0; i < attrs.getLength(); i++) {
Attr attr = (Attr) attrs.item(i);
String nsUri = "";
String prefix = attr.getPrefix();
String attrPrefix = attr.getPrefix();
if (attrPrefix == null) {
attrPrefix = NokogiriHelpers.getPrefix(attr.getNodeName());
}
String nodeName = attr.getNodeName();
if ("xml".equals(prefix)) {
nsUri = "http://www.w3.org/XML/1998/namespace";
} else if ("xmlns".equals(prefix) || nodeName.equals("xmlns")) {
} else if ("xmlns".equals(attrPrefix) || nodeName.equals("xmlns")) {
nsUri = "http://www.w3.org/2000/xmlns/";
} else {
nsUri = attr.getNamespaceURI();
nsUri = attr.lookupNamespaceURI(attrPrefix);
}
if (!(nsUri == null || "".equals(nsUri))) {
XmlNamespace.createFromAttr(context.getRuntime(), attr);
}
e.getOwnerDocument().renameNode(attr, nsUri, nodeName);
NokogiriHelpers.renameNode(attr, nsUri, nodeName);
}
}

if (e.hasChildNodes()) {
((XmlNodeSet) children(context)).relink_namespace(context);
if (this.node.hasChildNodes()) {
XmlNodeSet nodeSet = (XmlNodeSet)(children(context));
nodeSet.relink_namespace(context);
}
}
}
Expand Down Expand Up @@ -536,7 +551,7 @@ public void updateNodeNamespaceIfNecessary(ThreadContext context, XmlNamespace n
&& oldPrefix.equals(rubyStringToString(ns.prefix(context))));

if(update) {
this.node.getOwnerDocument().renameNode(this.node, uri, this.node.getNodeName());
this.node = NokogiriHelpers.renameNode(this.node, uri, this.node.getNodeName());
}
}

Expand Down Expand Up @@ -576,7 +591,7 @@ public IRubyObject add_namespace_definition(ThreadContext context,
else namespaceOwner = node.getParentNode();
XmlNamespace ns = XmlNamespace.createFromPrefixAndHref(namespaceOwner, prefix, href);
if (node != namespaceOwner) {
node.getOwnerDocument().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 @@ -1172,7 +1187,7 @@ public IRubyObject node_name(ThreadContext context) {
@JRubyMethod(name = {"node_name=", "name="})
public IRubyObject node_name_set(ThreadContext context, IRubyObject nodeName) {
String newName = rubyStringToString(nodeName);
getOwnerDocument().renameNode(node, null, newName);
this.node = NokogiriHelpers.renameNode(node, null, newName);
setName(nodeName);
return this;
}
Expand All @@ -1190,6 +1205,8 @@ public IRubyObject set(ThreadContext context, IRubyObject rbkey, IRubyObject rbv
String uri = null;
if (prefix.equals("xml")) {
uri = "http://www.w3.org/XML/1998/namespace";
} else if (prefix.equals("xmlns")) {
uri = "http://www.w3.org/2000/xmlns/";
} else {
uri = findNamespaceHref(context, prefix);
}
Expand All @@ -1214,7 +1231,11 @@ private String findNamespaceHref(ThreadContext context, String prefix) {
return namespace.getHref();
}
}
currentNode = (XmlNode) currentNode.parent(context);
if (currentNode.parent(context).isNil()) {
break;
} else {
currentNode = (XmlNode) currentNode.parent(context);
}
}
return null;
}
Expand Down Expand Up @@ -1261,7 +1282,7 @@ public IRubyObject set_namespace(ThreadContext context, IRubyObject namespace) {
String prefix = n.getPrefix();
String href = n.getNamespaceURI();
((XmlDocument)doc).getNamespaceCache().remove(prefix == null ? "" : prefix, href);
n.getOwnerDocument().renameNode(n, null, n.getNodeName());
this.node = NokogiriHelpers.renameNode(n, null, NokogiriHelpers.getLocalPart(n.getNodeName()));
}
} else {
XmlNamespace ns = (XmlNamespace) namespace;
Expand All @@ -1270,7 +1291,14 @@ public IRubyObject set_namespace(ThreadContext context, IRubyObject namespace) {

// Assigning node = ...renameNode() or not seems to make no
// difference. Why not? -pmahoney
node = node.getOwnerDocument().renameNode(node, href, NokogiriHelpers.newQName(prefix, node));

// It actually makes a great deal of difference. renameNode()
// will operate in place if it can, but sometimes it can't.
// The node you passed in *might* come back as you expect, but
// it might not. It's much safer to throw away the original
// and keep the return value. -mbklein
String new_name = NokogiriHelpers.newQName(prefix, node);
this.node = NokogiriHelpers.renameNode(node, href, new_name);
}

return this;
Expand Down Expand Up @@ -1485,7 +1513,7 @@ private void addNamespaceURIIfNeeded(Node child) {
XmlElement fragmentContext = ((XmlDocumentFragment)this).getFragmentContext();
String namespace_uri = fragmentContext.node.getNamespaceURI();
if (namespace_uri != null && namespace_uri.length() > 0) {
node.getOwnerDocument().renameNode(child, namespace_uri, child.getNodeName());
NokogiriHelpers.renameNode(child, namespace_uri, child.getNodeName());
}
}
}
Expand Down Expand Up @@ -1542,7 +1570,7 @@ protected void adoptAsReplacement(ThreadContext context,
try {
parentNode.replaceChild(otherNode, thisNode);
if (otherNode.getNodeType() != Node.TEXT_NODE) {
otherNode.getOwnerDocument().renameNode(otherNode, thisNode.getNamespaceURI(), otherNode.getNodeName());
NokogiriHelpers.renameNode(otherNode, thisNode.getNamespaceURI(), otherNode.getNodeName());
}
} catch (Exception e) {
String prefix = "could not replace child: ";
Expand Down
19 changes: 17 additions & 2 deletions ext/java/nokogiri/internals/NokogiriHelpers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,11 @@
import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.util.ByteList;
import org.w3c.dom.Attr;
import org.w3c.dom.Document;
import org.w3c.dom.NamedNodeMap;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import org.w3c.dom.DOMException;

/**
* A class for various utility methods.
Expand Down Expand Up @@ -634,10 +636,11 @@ public static String canonicalizeWhitespce(String s) {
}

public static String newQName(String newPrefix, Node node) {
String tagName = getLocalPart(node.getNodeName());
if(newPrefix == null) {
return node.getLocalName();
return tagName;
} else {
return newPrefix + ":" + node.getLocalName();
return newPrefix + ":" + tagName;
}
}

Expand Down Expand Up @@ -807,4 +810,16 @@ public static boolean shouldEncode(Node text) {
public static boolean shouldDecode(Node text) {
return !shouldEncode(text);
}

public static Node renameNode(Node n, String namespaceURI, String qualifiedName) throws DOMException {
Document doc = n.getOwnerDocument();
XmlDocument xmlDoc = (XmlDocument)getCachedNode(doc);
NokogiriNamespaceCache nsCache = xmlDoc.getNamespaceCache();
int oldHash = n.hashCode();
Node result = doc.renameNode(n, namespaceURI, qualifiedName);
if (result != n) {
nsCache.replaceNode(n, result);
}
return result;
}
}
19 changes: 18 additions & 1 deletion ext/java/nokogiri/internals/NokogiriNamespaceCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ 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.ownerNode == node) {
if (entry.isOwner(node)) {
namespaces.add(entry.namespace);
}
}
Expand Down Expand Up @@ -151,6 +151,15 @@ public void 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);
}
}
}

private class CacheEntry {
private XmlNamespace namespace;
private Node ownerNode;
Expand All @@ -159,5 +168,13 @@ private class CacheEntry {
this.namespace = namespace;
this.ownerNode = ownerNode;
}

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

public void replaceOwner(Node newNode) {
this.ownerNode = newNode;
}
}
}
33 changes: 32 additions & 1 deletion ext/nokogiri/xml_node.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,33 @@ typedef xmlNodePtr (*pivot_reparentee_func)(xmlNodePtr, xmlNodePtr);
/* :nodoc: */
static void relink_namespace(xmlNodePtr reparented)
{
xmlChar *name, *prefix;
xmlNodePtr child;
xmlNsPtr ns;

/* Also, don't bother relinking anything but elements and attributes */
if(reparented->type > 2) return;

if(reparented->ns == NULL || reparented->ns->prefix == NULL) {
name = xmlSplitQName2(reparented->name, &prefix);

if(reparented->type == 2) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block doesn't appear to have test coverage.

if (prefix == NULL || strcmp(prefix,"xmlns") == 0) return;
}

ns = xmlSearchNs(reparented->doc, reparented, prefix);

if (ns == NULL && reparented->parent)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block doesn't appear to have test coverage, either.

ns = xmlSearchNs(reparented->doc, reparented->parent, prefix);

if (ns != NULL) {
xmlNodeSetName(reparented, name);
xmlSetNs(reparented, ns);
}
}

/* Avoid segv when relinking against unlinked nodes. */
if(!reparented->parent) return;
if(reparented->type > 1 || !reparented->parent) return;

/* Make sure that our reparented node has the correct namespaces */
if(!reparented->ns && reparented->doc != (xmlDocPtr)reparented->parent)
Expand Down Expand Up @@ -70,6 +93,14 @@ static void relink_namespace(xmlNodePtr reparented)
relink_namespace(child);
child = child->next;
}

if (reparented->type == 1) {
child = (xmlNodePtr)((xmlElementPtr)reparented)->attributes;
while(NULL != child) {
relink_namespace(child);
child = child->next;
}
}
}

/* :nodoc: */
Expand Down
14 changes: 12 additions & 2 deletions lib/nokogiri/xml/builder.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,8 @@ def [] ns
return self if @ns
end

raise ArgumentError, "Namespace #{ns} has not been defined"
@ns = { :pending => ns.to_s }
return self
end

###
Expand Down Expand Up @@ -355,11 +356,20 @@ def method_missing method, *args, &block # :nodoc:
else
node = @doc.create_element(method.to_s.sub(/[_!]$/, ''),*args) { |n|
# Set up the namespace
if @ns
if @ns.is_a? Nokogiri::XML::Namespace
n.namespace = @ns
@ns = nil
end
}

if @ns.is_a? Hash
node.namespace = node.namespace_definitions.find { |x| x.prefix == @ns[:pending] }
if node.namespace.nil?
raise ArgumentError, "Namespace #{@ns[:pending]} has not been defined"
end
@ns = nil
end

insert(node, &block)
end
end
Expand Down
4 changes: 3 additions & 1 deletion lib/nokogiri/xml/document.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,16 @@ def create_element name, *args, &block
if key =~ NCNAME_RE
ns_name = key.split(":", 2)[1]
elm.add_namespace_definition ns_name, v
next
end
elm[k.to_s] = v.to_s
}
else
elm.content = arg
end
end
if ns = elm.namespace_definitions.find { |n| n.prefix.nil? or n.prefix == '' }
elm.namespace = ns
end
elm
end

Expand Down
Loading