Skip to content

Commit

Permalink
[refactor] node-set handling - avoid when unnecessary
Browse files Browse the repository at this point in the history 
just loop node-list children directly internally
make it explicit that node-set needs to initialize (@document)
  • Loading branch information
@kares
kares committed Oct 16, 2019
1 parent 77df21b commit 84a554f
Show file tree
Hide file tree
Showing 8 changed files with 121 additions and 126 deletions.
14 changes: 7 additions & 7 deletions ext/java/nokogiri/XmlDocument.java
View file
Expand Up @@ -338,13 +338,13 @@ public static IRubyObject read_memory(ThreadContext context, IRubyObject klass,

@JRubyMethod(name="remove_namespaces!")
public IRubyObject remove_namespaces(ThreadContext context) {
removeNamespceRecursively(context, this);
removeNamespaceRecursively(this);
if (nsCache != null) nsCache.clear();
clearXpathContext(getNode());
return this;
}

private void removeNamespceRecursively(ThreadContext context, XmlNode xmlNode) {
private void removeNamespaceRecursively(XmlNode xmlNode) {
Node node = xmlNode.node;
if (node.getNodeType() == Node.ELEMENT_NODE) {
node.setPrefix(null);
Expand All @@ -353,17 +353,17 @@ private void removeNamespceRecursively(ThreadContext context, XmlNode xmlNode) {
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);
((org.w3c.dom.Element) node).removeAttributeNode(attr);
} else {
attr.setPrefix(null);
NokogiriHelpers.renameNode(attr, null, attr.getLocalName());
}
}
}
XmlNodeSet nodeSet = (XmlNodeSet) xmlNode.children(context);
for (long i=0; i < nodeSet.length(); i++) {
XmlNode childNode = (XmlNode)nodeSet.slice(context, RubyFixnum.newFixnum(context.runtime, i));
removeNamespceRecursively(context, childNode);
IRubyObject[] nodes = xmlNode.getChildren();
for (int i=0; i < nodes.length; i++) {
XmlNode childNode = (XmlNode) nodes[i];
removeNamespaceRecursively(childNode);
}
}

Expand Down
2 changes: 1 addition & 1 deletion ext/java/nokogiri/XmlDocumentFragment.java
View file
Expand Up @@ -180,6 +180,6 @@ public XmlElement getFragmentContext() {

@Override
public void relink_namespace(ThreadContext context) {
((XmlNodeSet) children(context)).relink_namespace(context);
relink_namespace(context, getChildren());
}
}
4 changes: 2 additions & 2 deletions ext/java/nokogiri/XmlDtd.java
View file
Expand Up @@ -369,7 +369,7 @@ public static boolean isContentModel(Node node) {
* the various collections.
*/
protected void extractDecls(ThreadContext context) {
Ruby runtime = context.getRuntime();
Ruby runtime = context.runtime;

// initialize data structures
attributes = RubyHash.newHash(runtime);
Expand All @@ -383,7 +383,7 @@ protected void extractDecls(ThreadContext context) {
if (node == null) return; // leave all the decl hash's empty

// convert allDecls to a NodeSet
children = XmlNodeSet.newXmlNodeSet(context, extractDecls(context, node.getFirstChild()));
children = XmlNodeSet.newNodeSet(runtime, extractDecls(context, node.getFirstChild()));

// add attribute decls as attributes to the matching element decl
RubyArray keys = attributes.keys();
Expand Down
14 changes: 1 addition & 13 deletions ext/java/nokogiri/XmlElement.java
View file
Expand Up @@ -62,19 +62,7 @@ public XmlElement(Ruby runtime, RubyClass klazz, Node element) {
@Override
public void accept(ThreadContext context, SaveContextVisitor visitor) {
visitor.enter((Element) node);
XmlNodeSet xmlNodeSet = (XmlNodeSet) children(context);
if (xmlNodeSet.length() > 0) {
IRubyObject[] nodes = XmlNodeSet.getNodes(context, xmlNodeSet);
for( int i = 0; i < nodes.length; i++ ) {
Object item = nodes[i];
if (item instanceof XmlNode) {
((XmlNode) item).accept(context, visitor);
}
else if (item instanceof XmlNamespace) {
((XmlNamespace) item).accept(context, visitor);
}
}
}
acceptChildren(context, getChildren(), visitor);
visitor.leave((Element) node);
}
}
59 changes: 32 additions & 27 deletions ext/java/nokogiri/XmlNode.java
View file
Expand Up @@ -463,29 +463,36 @@ public void relink_namespace(ThreadContext context) {
}

if (this.node.hasChildNodes()) {
XmlNodeSet nodeSet = (XmlNodeSet)(children(context));
nodeSet.relink_namespace(context);
relink_namespace(context, getChildren());
}
}

static void relink_namespace(ThreadContext context, IRubyObject[] nodes) {
for (int i = 0; i < nodes.length; i++) {
if (nodes[i] instanceof XmlNode) {
((XmlNode) nodes[i]).relink_namespace(context);
}
}
}

// Users might extend XmlNode. This method works for such a case.
public void accept(ThreadContext context, SaveContextVisitor visitor) {
visitor.enter(node);
XmlNodeSet xmlNodeSet = (XmlNodeSet) children(context);
if (xmlNodeSet.length() > 0) {
RubyArray array = (RubyArray) xmlNodeSet.to_a(context);
for(int i = 0; i < array.getLength(); i++) {
Object item = array.get(i);
acceptChildren(context, getChildren(), visitor);
visitor.leave(node);
}

void acceptChildren(ThreadContext context, IRubyObject[] nodes, SaveContextVisitor visitor) {
if (nodes.length > 0) {
for (int i = 0; i < nodes.length; i++) {
Object item = nodes[i];
if (item instanceof XmlNode) {
XmlNode cur = (XmlNode) item;
cur.accept(context, visitor);
((XmlNode) item).accept(context, visitor);
} else if (item instanceof XmlNamespace) {
XmlNamespace cur = (XmlNamespace)item;
cur.accept(context, visitor);
((XmlNamespace) item).accept(context, visitor);
}
}
}
visitor.leave(node);
}

RubyString doSetName(IRubyObject name) {
Expand Down Expand Up @@ -642,17 +649,19 @@ public IRubyObject child(ThreadContext context) {

@JRubyMethod
public IRubyObject children(ThreadContext context) {
XmlNodeSet xmlNodeSet = XmlNodeSet.newEmptyNodeSet(context);
final IRubyObject[] nodes = getChildren();
if (nodes.length == 0) {
return XmlNodeSet.newEmptyNodeSet(context, this);
}
return XmlNodeSet.newNodeSet(context.runtime, nodes);
}

IRubyObject[] getChildren() {
NodeList nodeList = node.getChildNodes();
if (nodeList.getLength() > 0) {
xmlNodeSet.setNodeList(nodeList); // initializes @document from first node
}
else { // TODO this is very ripe for refactoring
setDocumentAndDecorate(context, xmlNodeSet, document(context.runtime));
return nodeListToRubyArray(getRuntime(), nodeList);
}

return xmlNodeSet;
return IRubyObject.NULL_ARRAY;
}

@JRubyMethod
Expand All @@ -677,8 +686,7 @@ public IRubyObject element_children(ThreadContext context) {
addElements(node, elementNodes, false);
IRubyObject[] array = NokogiriHelpers.nodeArrayToArray(context.runtime,
elementNodes.toArray(new Node[0]));
XmlNodeSet xmlNodeSet = XmlNodeSet.newXmlNodeSet(context, array);
return xmlNodeSet;
return XmlNodeSet.newNodeSet(context.runtime, array, this);
}

private void addElements(Node n, List<Node> nodes, boolean isFirstOnly) {
Expand Down Expand Up @@ -735,9 +743,7 @@ public IRubyObject compare(ThreadContext context, IRubyObject other) {
* <code>options</code> into account.
*/
@JRubyMethod(required = 2, visibility = Visibility.PRIVATE)
public IRubyObject in_context(ThreadContext context,
IRubyObject str,
IRubyObject options) {
public IRubyObject in_context(ThreadContext context, IRubyObject str, IRubyObject options) {
RubyClass klass;
XmlDomParserContext ctx;
InputStream istream;
Expand Down Expand Up @@ -776,7 +782,7 @@ public IRubyObject in_context(ThreadContext context,
documentErrors.append(docErrors.entry(i));
}
document.setInstanceVariable("@errors", documentErrors);
return XmlNodeSet.newXmlNodeSet(context, new IRubyObject[0]);
return XmlNodeSet.newNodeSet(context.runtime, IRubyObject.NULL_ARRAY, this);
}

// The first child might be document type node (dtd declaration).
Expand All @@ -789,8 +795,7 @@ public IRubyObject in_context(ThreadContext context,
}

IRubyObject[] nodes = new IRubyObject[] { NokogiriHelpers.getCachedNodeOrCreate(runtime, first) };
XmlNodeSet xmlNodeSet = XmlNodeSet.newXmlNodeSet(context, nodes);
return xmlNodeSet;
return XmlNodeSet.newNodeSet(context.runtime, nodes, this);
}

private static RubyArray getErrors(XmlDocument document) {
Expand Down

0 comments on commit 84a554f

Please sign in to comment.