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..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,13 @@ 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; + } + ParseData parseData = parse.getOrCreate(url); Metadata metadata = parseData.getMetadata(); @@ -84,7 +91,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; } @@ -114,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 84ab81f0f..7dd5c417f 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,13 +254,13 @@ 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; } } 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(