From 6bd1306a79c03342f03e850e8bd3858bc36ad6b2 Mon Sep 17 00:00:00 2001 From: Julien Nioche Date: Thu, 22 Oct 2026 11:05:17 +0100 Subject: [PATCH 1/2] #158 jsoup XPathFilter: convert to W3C DOM once per page, not once per expression Element.selectXpath() converts the whole document to a W3C DOM and creates a new XPathFactory on every call. XPathFilter and LinkParseFilter now build the W3C DOM once per page and compile their expressions at configuration time. Co-Authored-By: Claude Opus 5.5 --- .../stormcrawler/jsoup/LinkParseFilter.java | 7 ++- .../stormcrawler/jsoup/XPathFilter.java | 60 +++++++++++++++++-- 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/apache/stormcrawler/jsoup/LinkParseFilter.java b/core/src/main/java/org/apache/stormcrawler/jsoup/LinkParseFilter.java index 92a114f07..0d04dbf86 100644 --- a/core/src/main/java/org/apache/stormcrawler/jsoup/LinkParseFilter.java +++ b/core/src/main/java/org/apache/stormcrawler/jsoup/LinkParseFilter.java @@ -61,6 +61,11 @@ public class LinkParseFilter extends XPathFilter { public void filter( String url, byte[] content, org.jsoup.nodes.Document doc, ParseResult parse) { + W3CContext context = toW3C(url, doc); + if (context == null) { + return; + } + ParseData parseData = parse.getOrCreate(url); Metadata metadata = parseData.getMetadata(); @@ -84,7 +89,7 @@ public void filter( for (List leList : expressions.values()) { for (LabelledExpression le : leList) { try { - List values = le.evaluate(doc); + List values = le.evaluate(context); if (values == null || values.isEmpty()) { continue; } diff --git a/core/src/main/java/org/apache/stormcrawler/jsoup/XPathFilter.java b/core/src/main/java/org/apache/stormcrawler/jsoup/XPathFilter.java index 84ab81f0f..a45999af6 100644 --- a/core/src/main/java/org/apache/stormcrawler/jsoup/XPathFilter.java +++ b/core/src/main/java/org/apache/stormcrawler/jsoup/XPathFilter.java @@ -25,17 +25,23 @@ import java.util.Map.Entry; import java.util.regex.Matcher; import java.util.regex.Pattern; +import javax.xml.xpath.XPath; +import javax.xml.xpath.XPathConstants; +import javax.xml.xpath.XPathExpression; +import javax.xml.xpath.XPathExpressionException; +import javax.xml.xpath.XPathFactory; import org.apache.stormcrawler.Metadata; import org.apache.stormcrawler.parse.JSoupFilter; import org.apache.stormcrawler.parse.ParseData; import org.apache.stormcrawler.parse.ParseResult; import org.apache.stormcrawler.util.AbstractConfigurable; import org.jetbrains.annotations.NotNull; +import org.jsoup.helper.W3CDom; import org.jsoup.nodes.Document; import org.jsoup.nodes.Element; -import org.jsoup.select.Elements; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.w3c.dom.NodeList; /** Reads a XPATH patterns and stores the value found in web page as metadata. */ public class XPathFilter extends AbstractConfigurable implements JSoupFilter { @@ -44,6 +50,8 @@ public class XPathFilter extends AbstractConfigurable implements JSoupFilter { protected final Map> expressions = new HashMap<>(); + private final XPath xpath = XPathFactory.newInstance().newXPath(); + /** * Supported extraction functions that can be appended to XPath expressions. These provide * backward compatibility with the non-standard XSoup functions. @@ -98,16 +106,33 @@ public static String lowercaseElementNames(String xpath) { .replaceAll(m -> m.group().toLowerCase(java.util.Locale.ROOT)); } + /** + * W3C representation of a JSoup document, built once per page and shared by all the + * expressions. Element.selectXpath() would otherwise convert the whole document on every call. + */ + static class W3CContext { + + private final W3CDom w3c = new W3CDom().namespaceAware(false); + private final org.w3c.dom.Node contextNode; + + W3CContext(Document doc) { + this.contextNode = w3c.contextNode(w3c.fromJsoup(doc)); + } + } + static class LabelledExpression { String key; private String xpath; + private XPathExpression expression; private EvalFunction evalFunction; private String attrName; - private LabelledExpression(String key, String xpath) { + private LabelledExpression(String key, String xpath, XPath compiler) + throws XPathExpressionException { this.key = key; parseExpression(xpath); + this.expression = compiler.compile(this.xpath); } private void parseExpression(String rawXpath) { @@ -149,8 +174,10 @@ private void parseExpression(String rawXpath) { this.evalFunction = EvalFunction.NONE; } - List evaluate(Document doc) { - Elements elements = doc.selectXpath(xpath); + List evaluate(W3CContext context) throws XPathExpressionException { + NodeList nodes = + (NodeList) expression.evaluate(context.contextNode, XPathConstants.NODESET); + List elements = context.w3c.sourceNodes(nodes, Element.class); List results = new ArrayList<>(); for (Element element : elements) { String value = evalFunction.evaluate(element, attrName); @@ -189,16 +216,37 @@ private void addExpression(String key, JsonNode expression) { try { expressions .computeIfAbsent(key, k -> new ArrayList<>()) - .add(new LabelledExpression(key, xpathvalue)); + .add(new LabelledExpression(key, xpathvalue, xpath)); } catch (Exception e) { throw new RuntimeException("Can't compile expression : " + xpathvalue, e); } } + /** + * Converts the JSoup document into a W3C DOM against which the expressions are evaluated. + * Returns null if there are no expressions or the conversion failed. + */ + W3CContext toW3C(String url, Document doc) { + if (expressions.isEmpty()) { + return null; + } + try { + return new W3CContext(doc); + } catch (Exception e) { + LOG.error("Error converting {} to W3C DOM: {}", url, e); + return null; + } + } + @Override public void filter( String url, byte[] content, org.jsoup.nodes.Document doc, ParseResult parse) { + W3CContext context = toW3C(url, doc); + if (context == null) { + return; + } + ParseData parseData = parse.getOrCreate(url); Metadata metadata = parseData.getMetadata(); @@ -206,7 +254,7 @@ public void filter( for (List leList : expressions.values()) { for (LabelledExpression le : leList) { try { - List values = le.evaluate(doc); + List values = le.evaluate(context); if (values != null && !values.isEmpty()) { metadata.addValues(le.key, values); break; From abb97959fa060976e0d0d63a25c544bb9619b9fa Mon Sep 17 00:00:00 2001 From: Julien Nioche Date: Fri, 23 Oct 2026 07:01:42 +0100 Subject: [PATCH 2/2] #158 log exceptions with stack traces, test invalid expression in configure Address review comments: pass the exception to SLF4J as a throwable rather than filling a placeholder, document the early return in LinkParseFilter and check that an invalid XPath fails in configure(). Co-Authored-By: Claude Opus 5.5 --- .../apache/stormcrawler/jsoup/LinkParseFilter.java | 4 +++- .../org/apache/stormcrawler/jsoup/XPathFilter.java | 4 ++-- .../apache/stormcrawler/jsoup/JSoupFiltersTest.java | 13 +++++++++++++ 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/apache/stormcrawler/jsoup/LinkParseFilter.java b/core/src/main/java/org/apache/stormcrawler/jsoup/LinkParseFilter.java index 0d04dbf86..229275ece 100644 --- a/core/src/main/java/org/apache/stormcrawler/jsoup/LinkParseFilter.java +++ b/core/src/main/java/org/apache/stormcrawler/jsoup/LinkParseFilter.java @@ -61,6 +61,8 @@ public class LinkParseFilter extends XPathFilter { public void filter( String url, byte[] content, org.jsoup.nodes.Document doc, ParseResult parse) { + // nothing to add if there are no expressions or the conversion failed: + // leave the existing outlinks untouched W3CContext context = toW3C(url, doc); if (context == null) { return; @@ -119,7 +121,7 @@ public void filter( dedup.put(ol.getTargetURL(), ol); } } catch (Exception e) { - LOG.error("Error evaluating {}: {}", le.key, e); + LOG.error("Error evaluating {} on {}", le.key, url, e); } } } diff --git a/core/src/main/java/org/apache/stormcrawler/jsoup/XPathFilter.java b/core/src/main/java/org/apache/stormcrawler/jsoup/XPathFilter.java index a45999af6..7dd5c417f 100644 --- a/core/src/main/java/org/apache/stormcrawler/jsoup/XPathFilter.java +++ b/core/src/main/java/org/apache/stormcrawler/jsoup/XPathFilter.java @@ -233,7 +233,7 @@ W3CContext toW3C(String url, Document doc) { try { return new W3CContext(doc); } catch (Exception e) { - LOG.error("Error converting {} to W3C DOM: {}", url, e); + LOG.error("Error converting {} to W3C DOM", url, e); return null; } } @@ -260,7 +260,7 @@ public void filter( break; } } catch (Exception e) { - LOG.error("Error evaluating {}: {}", le.key, e); + LOG.error("Error evaluating {} on {}", le.key, url, e); } } } diff --git a/core/src/test/java/org/apache/stormcrawler/jsoup/JSoupFiltersTest.java b/core/src/test/java/org/apache/stormcrawler/jsoup/JSoupFiltersTest.java index 61afe49a5..b4d6b00b0 100644 --- a/core/src/test/java/org/apache/stormcrawler/jsoup/JSoupFiltersTest.java +++ b/core/src/test/java/org/apache/stormcrawler/jsoup/JSoupFiltersTest.java @@ -17,10 +17,13 @@ package org.apache.stormcrawler.jsoup; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; import java.io.IOException; import java.util.HashMap; import java.util.List; import java.util.Map; +import javax.xml.xpath.XPathExpressionException; import org.apache.storm.task.OutputCollector; import org.apache.stormcrawler.Metadata; import org.apache.stormcrawler.TestUtil; @@ -156,6 +159,16 @@ void testFallbackExpressions() throws IOException { Assertions.assertTrue(fallback.contains("Main Title")); } + @Test + void testInvalidExpressionFailsInConfigure() throws IOException { + JsonNode params = new ObjectMapper().readTree("{\"broken\": \"//div[@class=\"}"); + XPathFilter filter = new XPathFilter(); + RuntimeException e = + Assertions.assertThrows( + RuntimeException.class, () -> filter.configure(new HashMap<>(), params)); + Assertions.assertInstanceOf(XPathExpressionException.class, e.getCause()); + } + @Test void testLowercaseElementNames() { Assertions.assertEquals(