From ce21884a976a35d28fd0d1a3b8efd73c178a7595 Mon Sep 17 00:00:00 2001 From: Elias Dorneles Date: Mon, 3 Aug 2015 20:15:21 -0300 Subject: [PATCH 01/17] migrating scrapy Selector to use Parsel --- requirements.txt | 1 + scrapy/selector/unified.py | 167 +++---------------------------------- setup.py | 1 + tests/test_selector.py | 2 +- 4 files changed, 15 insertions(+), 156 deletions(-) diff --git a/requirements.txt b/requirements.txt index a05cd3680..23be40daa 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,3 +7,4 @@ queuelib six>=1.5.2 PyDispatcher>=2.0.5 service_identity +parsel>=0.9.0 diff --git a/scrapy/selector/unified.py b/scrapy/selector/unified.py index eed8f94f7..d00c1cd41 100644 --- a/scrapy/selector/unified.py +++ b/scrapy/selector/unified.py @@ -2,36 +2,18 @@ XPath selectors based on lxml """ -from lxml import etree -import six - -from scrapy.utils.misc import extract_regex from scrapy.utils.trackref import object_ref -from scrapy.utils.python import to_bytes, flatten, iflatten -from scrapy.utils.decorators import deprecated +from scrapy.utils.python import to_bytes from scrapy.http import HtmlResponse, XmlResponse +from scrapy.utils.decorators import deprecated +from parsel import Selector as ParselSelector, SelectorList +from parsel.unified import _ctgroup from .lxmldocument import LxmlDocument -from .csstranslator import ScrapyHTMLTranslator, ScrapyGenericTranslator __all__ = ['Selector', 'SelectorList'] -class SafeXMLParser(etree.XMLParser): - def __init__(self, *args, **kwargs): - kwargs.setdefault('resolve_entities', False) - super(SafeXMLParser, self).__init__(*args, **kwargs) - -_ctgroup = { - 'html': {'_parser': etree.HTMLParser, - '_csstranslator': ScrapyHTMLTranslator(), - '_tostring_method': 'html'}, - 'xml': {'_parser': SafeXMLParser, - '_csstranslator': ScrapyGenericTranslator(), - '_tostring_method': 'xml'}, -} - - def _st(response, st): if st is None: return 'xml' if isinstance(response, XmlResponse) else 'html' @@ -47,111 +29,25 @@ def _response_from_text(text, st): body=to_bytes(text, 'utf-8')) -class Selector(object_ref): +class Selector(ParselSelector, object_ref): - __slots__ = ['response', 'text', 'namespaces', 'type', '_expr', '_root', - '__weakref__', '_parser', '_csstranslator', '_tostring_method'] + __slots__ = ['response'] - _default_type = None - _default_namespaces = { - "re": "http://exslt.org/regular-expressions", + def __init__(self, response=None, text=None, type=None, root=None, **kwargs): + st = _st(response, type or self._default_type) + root = kwargs.get('root', root) - # supported in libxslt: - # set:difference - # set:has-same-node - # set:intersection - # set:leading - # set:trailing - "set": "http://exslt.org/sets" - } - _lxml_smart_strings = False - - def __init__(self, response=None, text=None, type=None, namespaces=None, - _root=None, _expr=None): - self.type = st = _st(response, type or self._default_type) self._parser = _ctgroup[st]['_parser'] - self._csstranslator = _ctgroup[st]['_csstranslator'] - self._tostring_method = _ctgroup[st]['_tostring_method'] if text is not None: response = _response_from_text(text, st) if response is not None: - _root = LxmlDocument(response, self._parser) + root = LxmlDocument(response, self._parser) self.response = response - self.namespaces = dict(self._default_namespaces) - if namespaces is not None: - self.namespaces.update(namespaces) - self._root = _root - self._expr = _expr - - def xpath(self, query): - try: - xpathev = self._root.xpath - except AttributeError: - return SelectorList([]) - - try: - result = xpathev(query, namespaces=self.namespaces, - smart_strings=self._lxml_smart_strings) - except etree.XPathError: - msg = u"Invalid XPath: %s" % query - raise ValueError(msg if six.PY3 else msg.encode("unicode_escape")) - - if type(result) is not list: - result = [result] - - result = [self.__class__(_root=x, _expr=query, - namespaces=self.namespaces, - type=self.type) - for x in result] - return SelectorList(result) - - def css(self, query): - return self.xpath(self._css2xpath(query)) - - def _css2xpath(self, query): - return self._csstranslator.css_to_xpath(query) - - def re(self, regex): - return extract_regex(regex, self.extract()) - - def extract(self): - try: - return etree.tostring(self._root, - method=self._tostring_method, - encoding="unicode", - with_tail=False) - except (AttributeError, TypeError): - if self._root is True: - return u'1' - elif self._root is False: - return u'0' - else: - return six.text_type(self._root) - - def register_namespace(self, prefix, uri): - if self.namespaces is None: - self.namespaces = {} - self.namespaces[prefix] = uri - - def remove_namespaces(self): - for el in self._root.iter('*'): - if el.tag.startswith('{'): - el.tag = el.tag.split('}', 1)[1] - # loop on element attributes also - for an in el.attrib.keys(): - if an.startswith('{'): - el.attrib[an.split('}', 1)[1]] = el.attrib.pop(an) - - def __nonzero__(self): - return bool(self.extract()) - - def __str__(self): - data = repr(self.extract()[:40]) - return "<%s xpath=%r data=%s>" % (type(self).__name__, self._expr, data) - __repr__ = __str__ + text = response.body_as_unicode() if response else None + super(Selector, self).__init__(text=text, type=st, root=root, **kwargs) # Deprecated api @deprecated(use_instead='.xpath()') @@ -162,42 +58,3 @@ class Selector(object_ref): def extract_unquoted(self): return self.extract() - -class SelectorList(list): - - def __getslice__(self, i, j): - return self.__class__(list.__getslice__(self, i, j)) - - def xpath(self, xpath): - return self.__class__(flatten([x.xpath(xpath) for x in self])) - - def css(self, xpath): - return self.__class__(flatten([x.css(xpath) for x in self])) - - def re(self, regex): - return flatten([x.re(regex) for x in self]) - - def re_first(self, regex): - for el in iflatten(x.re(regex) for x in self): - return el - - def extract(self): - return [x.extract() for x in self] - - def extract_first(self, default=None): - for x in self: - return x.extract() - else: - return default - - @deprecated(use_instead='.extract()') - def extract_unquoted(self): - return [x.extract_unquoted() for x in self] - - @deprecated(use_instead='.xpath()') - def x(self, xpath): - return self.select(xpath) - - @deprecated(use_instead='.xpath()') - def select(self, xpath): - return self.xpath(xpath) diff --git a/setup.py b/setup.py index 7214f0dc6..959a73517 100644 --- a/setup.py +++ b/setup.py @@ -44,6 +44,7 @@ setup( 'pyOpenSSL', 'cssselect>=0.9', 'six>=1.5.2', + 'parsel>=0.9.0', 'PyDispatcher>=2.0.5', 'service_identity', ], diff --git a/tests/test_selector.py b/tests/test_selector.py index ad6a0e21c..02f82298c 100644 --- a/tests/test_selector.py +++ b/tests/test_selector.py @@ -332,7 +332,7 @@ class SelectorTestCase(unittest.TestCase): def test_weakref_slots(self): """Check that classes are using slots and are weak-referenceable""" - x = self.sscls() + x = self.sscls(text='') weakref.ref(x) assert not hasattr(x, '__dict__'), "%s does not use __slots__" % \ x.__class__.__name__ From c7b29d118d97b6fb510ba5b6ae3fb8eea41196c0 Mon Sep 17 00:00:00 2001 From: Elias Dorneles Date: Mon, 3 Aug 2015 20:37:42 -0300 Subject: [PATCH 02/17] fix support to legacy _root argument --- scrapy/selector/unified.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scrapy/selector/unified.py b/scrapy/selector/unified.py index d00c1cd41..9e0716671 100644 --- a/scrapy/selector/unified.py +++ b/scrapy/selector/unified.py @@ -35,7 +35,9 @@ class Selector(ParselSelector, object_ref): def __init__(self, response=None, text=None, type=None, root=None, **kwargs): st = _st(response, type or self._default_type) - root = kwargs.get('root', root) + + # supporting legacy _root argument + root = kwargs.get('_root', root) self._parser = _ctgroup[st]['_parser'] From 3a572e2f3b494327c918c2ac4a7150727c448f30 Mon Sep 17 00:00:00 2001 From: Elias Dorneles Date: Mon, 3 Aug 2015 20:38:15 -0300 Subject: [PATCH 03/17] cleanup csstranslator module, keeping only imports --- scrapy/selector/csstranslator.py | 94 ++------------------------------ 1 file changed, 6 insertions(+), 88 deletions(-) diff --git a/scrapy/selector/csstranslator.py b/scrapy/selector/csstranslator.py index 7482837a0..dfeb00be0 100644 --- a/scrapy/selector/csstranslator.py +++ b/scrapy/selector/csstranslator.py @@ -1,88 +1,6 @@ -from cssselect import GenericTranslator, HTMLTranslator -from cssselect.xpath import _unicode_safe_getattr, XPathExpr, ExpressionError -from cssselect.parser import FunctionalPseudoElement - - -class ScrapyXPathExpr(XPathExpr): - - textnode = False - attribute = None - - @classmethod - def from_xpath(cls, xpath, textnode=False, attribute=None): - x = cls(path=xpath.path, element=xpath.element, condition=xpath.condition) - x.textnode = textnode - x.attribute = attribute - return x - - def __str__(self): - path = super(ScrapyXPathExpr, self).__str__() - if self.textnode: - if path == '*': - path = 'text()' - elif path.endswith('::*/*'): - path = path[:-3] + 'text()' - else: - path += '/text()' - - if self.attribute is not None: - if path.endswith('::*/*'): - path = path[:-2] - path += '/@%s' % self.attribute - - return path - - def join(self, combiner, other): - super(ScrapyXPathExpr, self).join(combiner, other) - self.textnode = other.textnode - self.attribute = other.attribute - return self - - -class TranslatorMixin(object): - - def xpath_element(self, selector): - xpath = super(TranslatorMixin, self).xpath_element(selector) - return ScrapyXPathExpr.from_xpath(xpath) - - def xpath_pseudo_element(self, xpath, pseudo_element): - if isinstance(pseudo_element, FunctionalPseudoElement): - method = 'xpath_%s_functional_pseudo_element' % ( - pseudo_element.name.replace('-', '_')) - method = _unicode_safe_getattr(self, method, None) - if not method: - raise ExpressionError( - "The functional pseudo-element ::%s() is unknown" - % pseudo_element.name) - xpath = method(xpath, pseudo_element) - else: - method = 'xpath_%s_simple_pseudo_element' % ( - pseudo_element.replace('-', '_')) - method = _unicode_safe_getattr(self, method, None) - if not method: - raise ExpressionError( - "The pseudo-element ::%s is unknown" - % pseudo_element) - xpath = method(xpath) - return xpath - - def xpath_attr_functional_pseudo_element(self, xpath, function): - if function.argument_types() not in (['STRING'], ['IDENT']): - raise ExpressionError( - "Expected a single string or ident for ::attr(), got %r" - % function.arguments) - return ScrapyXPathExpr.from_xpath(xpath, - attribute=function.arguments[0].value) - - def xpath_text_simple_pseudo_element(self, xpath): - """Support selecting text nodes using ::text pseudo-element""" - return ScrapyXPathExpr.from_xpath(xpath, textnode=True) - - -class ScrapyGenericTranslator(TranslatorMixin, GenericTranslator): - pass - - -class ScrapyHTMLTranslator(TranslatorMixin, HTMLTranslator): - pass - +from parsel.csstranslator import ( + ScrapyXPathExpr, + TranslatorMixin, + ScrapyGenericTranslator, + ScrapyHTMLTranslator +) From 01d948f0fd792c3700306efe1bd38ae2163cb042 Mon Sep 17 00:00:00 2001 From: Elias Dorneles Date: Tue, 4 Aug 2015 16:28:02 -0300 Subject: [PATCH 04/17] remove selector support for LxmlDocument DOM cache and add deprecation warning for _root argument --- scrapy/selector/unified.py | 20 +++++++++----------- tests/test_selector.py | 11 +++++++++++ 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/scrapy/selector/unified.py b/scrapy/selector/unified.py index 9e0716671..db2a8510a 100644 --- a/scrapy/selector/unified.py +++ b/scrapy/selector/unified.py @@ -2,13 +2,13 @@ XPath selectors based on lxml """ +import warnings +from parsel import Selector as ParselSelector, SelectorList from scrapy.utils.trackref import object_ref from scrapy.utils.python import to_bytes from scrapy.http import HtmlResponse, XmlResponse from scrapy.utils.decorators import deprecated -from parsel import Selector as ParselSelector, SelectorList -from parsel.unified import _ctgroup -from .lxmldocument import LxmlDocument +from scrapy.exceptions import ScrapyDeprecationWarning __all__ = ['Selector', 'SelectorList'] @@ -33,22 +33,21 @@ class Selector(ParselSelector, object_ref): __slots__ = ['response'] - def __init__(self, response=None, text=None, type=None, root=None, **kwargs): + def __init__(self, response=None, text=None, type=None, root=None, _root=None, **kwargs): st = _st(response, type or self._default_type) - # supporting legacy _root argument - root = kwargs.get('_root', root) - - self._parser = _ctgroup[st]['_parser'] + if root is None and _root is not None: + warnings.warn("Argument `_root` is deprecated, use `root` instead", + ScrapyDeprecationWarning, stacklevel=2) + root = _root if text is not None: response = _response_from_text(text, st) if response is not None: - root = LxmlDocument(response, self._parser) + text = response.body_as_unicode() self.response = response - text = response.body_as_unicode() if response else None super(Selector, self).__init__(text=text, type=st, root=root, **kwargs) # Deprecated api @@ -59,4 +58,3 @@ class Selector(ParselSelector, object_ref): @deprecated(use_instead='.extract()') def extract_unquoted(self): return self.extract() - diff --git a/tests/test_selector.py b/tests/test_selector.py index 02f82298c..dd27ae7da 100644 --- a/tests/test_selector.py +++ b/tests/test_selector.py @@ -7,6 +7,8 @@ from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import TextResponse, HtmlResponse, XmlResponse from scrapy.selector import Selector from scrapy.selector.lxmlsel import XmlXPathSelector, HtmlXPathSelector, XPathSelector +from lxml import etree +from tests import mock class SelectorTestCase(unittest.TestCase): @@ -37,6 +39,15 @@ class SelectorTestCase(unittest.TestCase): self.assertEqual([x.extract() for x in sel.xpath("concat(//input[@name='a']/@value, //input[@name='b']/@value)")], [u'12']) + @mock.patch('scrapy.selector.unified.warnings') + def test_deprecated_root_argument(self, warnings): + root = etree.fromstring(u'') + sel = self.sscls(_root=root) + self.assertEqual(root, sel._root) + warnings.warn.assert_called_once_with( + 'Argument `_root` is deprecated, use `root` instead', + mock.ANY, stacklevel=2) + def test_representation_slice(self): body = u"

".format(50 * 'b') response = TextResponse(url="http://example.com", body=body, encoding='utf8') From 17d7347a3698ac7e3650f539578777d63c933262 Mon Sep 17 00:00:00 2001 From: Elias Dorneles Date: Wed, 5 Aug 2015 16:57:41 -0300 Subject: [PATCH 05/17] update minimal parsel version, add deprecated classes for csstranslator modules, fix test --- requirements.txt | 2 +- scrapy/selector/csstranslator.py | 21 +++++++++++++++------ setup.py | 2 +- tests/test_selector.py | 2 +- 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/requirements.txt b/requirements.txt index 23be40daa..ffa5b3025 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,4 +7,4 @@ queuelib six>=1.5.2 PyDispatcher>=2.0.5 service_identity -parsel>=0.9.0 +parsel>=0.9.1 diff --git a/scrapy/selector/csstranslator.py b/scrapy/selector/csstranslator.py index dfeb00be0..8d7f034af 100644 --- a/scrapy/selector/csstranslator.py +++ b/scrapy/selector/csstranslator.py @@ -1,6 +1,15 @@ -from parsel.csstranslator import ( - ScrapyXPathExpr, - TranslatorMixin, - ScrapyGenericTranslator, - ScrapyHTMLTranslator -) +from parsel.csstranslator import XPathExpr, GenericTranslator, HTMLTranslator +from scrapy.utils.deprecate import create_deprecated_class + + +ScrapyXPathExpr = create_deprecated_class( + 'ScrapyXPathExpr', XPathExpr, + new_class_path='parsel.csstranslator.XPathExpr') + +ScrapyGenericTranslator = create_deprecated_class( + 'ScrapyGenericTranslator', GenericTranslator, + new_class_path='parsel.csstranslator.GenericTranslator') + +ScrapyHTMLTranslator = create_deprecated_class( + 'ScrapyHTMLTranslator', HTMLTranslator, + new_class_path='parsel.csstranslator.HTMLTranslator') diff --git a/setup.py b/setup.py index 959a73517..6fe303f15 100644 --- a/setup.py +++ b/setup.py @@ -44,7 +44,7 @@ setup( 'pyOpenSSL', 'cssselect>=0.9', 'six>=1.5.2', - 'parsel>=0.9.0', + 'parsel>=0.9.1', 'PyDispatcher>=2.0.5', 'service_identity', ], diff --git a/tests/test_selector.py b/tests/test_selector.py index dd27ae7da..8dc194e19 100644 --- a/tests/test_selector.py +++ b/tests/test_selector.py @@ -43,7 +43,7 @@ class SelectorTestCase(unittest.TestCase): def test_deprecated_root_argument(self, warnings): root = etree.fromstring(u'') sel = self.sscls(_root=root) - self.assertEqual(root, sel._root) + self.assertIs(root, sel._root) warnings.warn.assert_called_once_with( 'Argument `_root` is deprecated, use `root` instead', mock.ANY, stacklevel=2) From 35c1dcdbc2eeaace7fce5e58e22f395625b87422 Mon Sep 17 00:00:00 2001 From: Elias Dorneles Date: Wed, 5 Aug 2015 19:47:16 -0300 Subject: [PATCH 06/17] use response.selector in link extractors instead of instantiating new Selector --- scrapy/linkextractors/lxmlhtml.py | 8 +++----- scrapy/linkextractors/sgml.py | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/scrapy/linkextractors/lxmlhtml.py b/scrapy/linkextractors/lxmlhtml.py index c952a5f83..7c41a88ff 100644 --- a/scrapy/linkextractors/lxmlhtml.py +++ b/scrapy/linkextractors/lxmlhtml.py @@ -65,9 +65,8 @@ class LxmlParserLinkExtractor(object): if self.unique else links def extract_links(self, response): - html = Selector(response) base_url = get_base_url(response) - return self._extract_links(html, response.url, response.encoding, base_url) + return self._extract_links(response.selector, response.url, response.encoding, base_url) def _process_links(self, links): """ Normalize and filter extracted links @@ -95,14 +94,13 @@ class LxmlLinkExtractor(FilteringLinkExtractor): canonicalize=canonicalize, deny_extensions=deny_extensions) def extract_links(self, response): - html = Selector(response) base_url = get_base_url(response) if self.restrict_xpaths: docs = [subdoc for x in self.restrict_xpaths - for subdoc in html.xpath(x)] + for subdoc in response.xpath(x)] else: - docs = [html] + docs = [response.selector] all_links = [] for doc in docs: links = self._extract_links(doc, response.url, response.encoding, base_url) diff --git a/scrapy/linkextractors/sgml.py b/scrapy/linkextractors/sgml.py index e4c2c274f..d045baa24 100644 --- a/scrapy/linkextractors/sgml.py +++ b/scrapy/linkextractors/sgml.py @@ -127,7 +127,7 @@ class SgmlLinkExtractor(FilteringLinkExtractor): def extract_links(self, response): base_url = None if self.restrict_xpaths: - sel = Selector(response) + sel = response.selector base_url = get_base_url(response) body = u''.join(f for x in self.restrict_xpaths From 6287fc310948d15869c14fff9781800fd3ccc3c7 Mon Sep 17 00:00:00 2001 From: Elias Dorneles Date: Thu, 6 Aug 2015 21:55:05 -0300 Subject: [PATCH 07/17] remove lxmldocument dependency from http.request.form --- scrapy/http/request/form.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/scrapy/http/request/form.py b/scrapy/http/request/form.py index 0d37004fb..ad3f0571e 100644 --- a/scrapy/http/request/form.py +++ b/scrapy/http/request/form.py @@ -7,6 +7,7 @@ See documentation in docs/topics/request-response.rst from six.moves.urllib.parse import urljoin, urlencode import lxml.html +from lxml import etree import six from scrapy.http.request import Request from scrapy.utils.python import to_bytes, is_listlike @@ -54,10 +55,15 @@ def _urlencode(seq, enc): return urlencode(values, doseq=1) +def _create_parser_from_response(response, parser_cls): + body = response.body_as_unicode().strip().encode('utf8') or b'' + parser = parser_cls(recover=True, encoding='utf8') + return etree.fromstring(body, parser=parser, base_url=response.url) + + def _get_form(response, formname, formid, formnumber, formxpath): """Find the form element """ - from scrapy.selector.lxmldocument import LxmlDocument - root = LxmlDocument(response, lxml.html.HTMLParser) + root = _create_parser_from_response(response, lxml.html.HTMLParser) forms = root.xpath('//form') if not forms: raise ValueError("No
element found in %s" % response) From 94c3a345b75db3e28dc0cca565650008d69e8992 Mon Sep 17 00:00:00 2001 From: Elias Dorneles Date: Thu, 6 Aug 2015 22:35:43 -0300 Subject: [PATCH 08/17] remove deprecated module lxmldocument --- scrapy/selector/lxmldocument.py | 31 ----------------------------- tests/py3-ignores.txt | 1 - tests/test_selector_lxmldocument.py | 26 ------------------------ 3 files changed, 58 deletions(-) delete mode 100644 scrapy/selector/lxmldocument.py delete mode 100644 tests/test_selector_lxmldocument.py diff --git a/scrapy/selector/lxmldocument.py b/scrapy/selector/lxmldocument.py deleted file mode 100644 index 817349b58..000000000 --- a/scrapy/selector/lxmldocument.py +++ /dev/null @@ -1,31 +0,0 @@ -""" -This module contains a simple class (LxmlDocument) which provides cache and -garbage collection to lxml element tree documents. -""" - -import weakref -from lxml import etree -from scrapy.utils.trackref import object_ref - - -def _factory(response, parser_cls): - url = response.url - body = response.body_as_unicode().strip().encode('utf8') or '' - parser = parser_cls(recover=True, encoding='utf8') - return etree.fromstring(body, parser=parser, base_url=url) - - -class LxmlDocument(object_ref): - - cache = weakref.WeakKeyDictionary() - __slots__ = ['__weakref__'] - - def __new__(cls, response, parser=etree.HTMLParser): - cache = cls.cache.setdefault(response, {}) - if parser not in cache: - obj = object_ref.__new__(cls) - cache[parser] = _factory(response, parser) - return cache[parser] - - def __str__(self): - return "" % self.root.tag diff --git a/tests/py3-ignores.txt b/tests/py3-ignores.txt index 9be3a99a8..8f5c0de48 100644 --- a/tests/py3-ignores.txt +++ b/tests/py3-ignores.txt @@ -26,7 +26,6 @@ tests/test_pipeline_files.py tests/test_pipeline_images.py tests/test_proxy_connect.py tests/test_selector_csstranslator.py -tests/test_selector_lxmldocument.py tests/test_selector.py tests/test_spidermiddleware_depth.py tests/test_spidermiddleware_httperror.py diff --git a/tests/test_selector_lxmldocument.py b/tests/test_selector_lxmldocument.py deleted file mode 100644 index 090cc21bc..000000000 --- a/tests/test_selector_lxmldocument.py +++ /dev/null @@ -1,26 +0,0 @@ -import unittest -from scrapy.selector.lxmldocument import LxmlDocument -from scrapy.http import TextResponse, HtmlResponse - - -class LxmlDocumentTest(unittest.TestCase): - - def test_caching(self): - r1 = HtmlResponse('http://www.example.com', body=b'') - r2 = r1.copy() - - doc1 = LxmlDocument(r1) - doc2 = LxmlDocument(r1) - doc3 = LxmlDocument(r2) - - # make sure it's cached - assert doc1 is doc2 - assert doc1 is not doc3 - - def test_null_char(self): - # make sure bodies with null char ('\x00') don't raise a TypeError exception - body = b'test problematic \x00 body' - response = TextResponse('http://example.com/catalog/product/blabla-123', - headers={'Content-Type': 'text/plain; charset=utf-8'}, - body=body) - LxmlDocument(response) From 67c98b185b4ee2a55c71dd1c0266b200fd3bcc04 Mon Sep 17 00:00:00 2001 From: Elias Dorneles Date: Thu, 6 Aug 2015 23:31:06 -0300 Subject: [PATCH 09/17] avoid harcoded check for selector type --- scrapy/selector/unified.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/scrapy/selector/unified.py b/scrapy/selector/unified.py index db2a8510a..3c75a92d2 100644 --- a/scrapy/selector/unified.py +++ b/scrapy/selector/unified.py @@ -17,10 +17,7 @@ __all__ = ['Selector', 'SelectorList'] def _st(response, st): if st is None: return 'xml' if isinstance(response, XmlResponse) else 'html' - elif st in ('xml', 'html'): - return st - else: - raise ValueError('Invalid type: %s' % st) + return st def _response_from_text(text, st): From 2fe6d128f51f94f55ac9d83ac05eb9c763c478b8 Mon Sep 17 00:00:00 2001 From: Elias Dorneles Date: Fri, 7 Aug 2015 15:26:54 -0300 Subject: [PATCH 10/17] upgrade parsel and using promoted root attribute --- requirements.txt | 2 +- scrapy/linkextractors/lxmlhtml.py | 2 +- scrapy/selector/unified.py | 6 ++++++ setup.py | 2 +- tests/test_selector.py | 2 +- 5 files changed, 10 insertions(+), 4 deletions(-) diff --git a/requirements.txt b/requirements.txt index ffa5b3025..f8a0f7ad3 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,4 +7,4 @@ queuelib six>=1.5.2 PyDispatcher>=2.0.5 service_identity -parsel>=0.9.1 +parsel>=0.9.2 diff --git a/scrapy/linkextractors/lxmlhtml.py b/scrapy/linkextractors/lxmlhtml.py index 7c41a88ff..606a45212 100644 --- a/scrapy/linkextractors/lxmlhtml.py +++ b/scrapy/linkextractors/lxmlhtml.py @@ -47,7 +47,7 @@ class LxmlParserLinkExtractor(object): def _extract_links(self, selector, response_url, response_encoding, base_url): links = [] # hacky way to get the underlying lxml parsed document - for el, attr, attr_val in self._iter_links(selector._root): + for el, attr, attr_val in self._iter_links(selector.root): # pseudo lxml.html.HtmlElement.make_links_absolute(base_url) attr_val = urljoin(base_url, attr_val) url = self.process_attr(attr_val) diff --git a/scrapy/selector/unified.py b/scrapy/selector/unified.py index 3c75a92d2..e229b10b8 100644 --- a/scrapy/selector/unified.py +++ b/scrapy/selector/unified.py @@ -48,6 +48,12 @@ class Selector(ParselSelector, object_ref): super(Selector, self).__init__(text=text, type=st, root=root, **kwargs) # Deprecated api + @property + def _root(self): + warnings.warn("Attribute `_root` is deprecated, use `root` instead", + ScrapyDeprecationWarning, stacklevel=2) + return self.root + @deprecated(use_instead='.xpath()') def select(self, xpath): return self.xpath(xpath) diff --git a/setup.py b/setup.py index 6fe303f15..2d2d8660f 100644 --- a/setup.py +++ b/setup.py @@ -44,7 +44,7 @@ setup( 'pyOpenSSL', 'cssselect>=0.9', 'six>=1.5.2', - 'parsel>=0.9.1', + 'parsel>=0.9.2', 'PyDispatcher>=2.0.5', 'service_identity', ], diff --git a/tests/test_selector.py b/tests/test_selector.py index 8dc194e19..d9660c674 100644 --- a/tests/test_selector.py +++ b/tests/test_selector.py @@ -43,7 +43,7 @@ class SelectorTestCase(unittest.TestCase): def test_deprecated_root_argument(self, warnings): root = etree.fromstring(u'') sel = self.sscls(_root=root) - self.assertIs(root, sel._root) + self.assertIs(root, sel.root) warnings.warn.assert_called_once_with( 'Argument `_root` is deprecated, use `root` instead', mock.ANY, stacklevel=2) From 26ebccd37a22e15eb522d6e7d979fd816ebbcdd8 Mon Sep 17 00:00:00 2001 From: Elias Dorneles Date: Fri, 7 Aug 2015 17:33:59 -0300 Subject: [PATCH 11/17] upgrade parsel and use its function to instantiate root for finding form --- requirements.txt | 2 +- scrapy/http/request/form.py | 11 +++-------- setup.py | 2 +- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/requirements.txt b/requirements.txt index f8a0f7ad3..afefb9d33 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,4 +7,4 @@ queuelib six>=1.5.2 PyDispatcher>=2.0.5 service_identity -parsel>=0.9.2 +parsel>=0.9.3 diff --git a/scrapy/http/request/form.py b/scrapy/http/request/form.py index ad3f0571e..0b1d3b926 100644 --- a/scrapy/http/request/form.py +++ b/scrapy/http/request/form.py @@ -7,7 +7,7 @@ See documentation in docs/topics/request-response.rst from six.moves.urllib.parse import urljoin, urlencode import lxml.html -from lxml import etree +from parsel.selector import create_root_node import six from scrapy.http.request import Request from scrapy.utils.python import to_bytes, is_listlike @@ -55,15 +55,10 @@ def _urlencode(seq, enc): return urlencode(values, doseq=1) -def _create_parser_from_response(response, parser_cls): - body = response.body_as_unicode().strip().encode('utf8') or b'' - parser = parser_cls(recover=True, encoding='utf8') - return etree.fromstring(body, parser=parser, base_url=response.url) - - def _get_form(response, formname, formid, formnumber, formxpath): """Find the form element """ - root = _create_parser_from_response(response, lxml.html.HTMLParser) + text = response.body_as_unicode() + root = create_root_node(text, lxml.html.HTMLParser, base_url=response.url) forms = root.xpath('//form') if not forms: raise ValueError("No element found in %s" % response) diff --git a/setup.py b/setup.py index 2d2d8660f..4eb8d2318 100644 --- a/setup.py +++ b/setup.py @@ -44,7 +44,7 @@ setup( 'pyOpenSSL', 'cssselect>=0.9', 'six>=1.5.2', - 'parsel>=0.9.2', + 'parsel>=0.9.3', 'PyDispatcher>=2.0.5', 'service_identity', ], From 12579b9afa20612734355c6d0d10481857dbd184 Mon Sep 17 00:00:00 2001 From: Elias Dorneles Date: Sun, 9 Aug 2015 01:21:39 -0300 Subject: [PATCH 12/17] warning when ambiguous root arguments and minor cleanups --- scrapy/linkextractors/sgml.py | 3 +-- scrapy/selector/unified.py | 7 +++++-- tests/test_selector.py | 24 +++++++++++++++--------- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/scrapy/linkextractors/sgml.py b/scrapy/linkextractors/sgml.py index d045baa24..4a6a24254 100644 --- a/scrapy/linkextractors/sgml.py +++ b/scrapy/linkextractors/sgml.py @@ -127,11 +127,10 @@ class SgmlLinkExtractor(FilteringLinkExtractor): def extract_links(self, response): base_url = None if self.restrict_xpaths: - sel = response.selector base_url = get_base_url(response) body = u''.join(f for x in self.restrict_xpaths - for f in sel.xpath(x).extract() + for f in response.xpath(x).extract() ).encode(response.encoding, errors='xmlcharrefreplace') else: body = response.body diff --git a/scrapy/selector/unified.py b/scrapy/selector/unified.py index e229b10b8..a8f80c84d 100644 --- a/scrapy/selector/unified.py +++ b/scrapy/selector/unified.py @@ -33,10 +33,13 @@ class Selector(ParselSelector, object_ref): def __init__(self, response=None, text=None, type=None, root=None, _root=None, **kwargs): st = _st(response, type or self._default_type) - if root is None and _root is not None: + if _root is not None: warnings.warn("Argument `_root` is deprecated, use `root` instead", ScrapyDeprecationWarning, stacklevel=2) - root = _root + if root is None: + root = _root + else: + warnings.warn("Ignoring deprecated `_root` argument, using provided `root`") if text is not None: response = _response_from_text(text, st) diff --git a/tests/test_selector.py b/tests/test_selector.py index d9660c674..dc37da86b 100644 --- a/tests/test_selector.py +++ b/tests/test_selector.py @@ -8,7 +8,6 @@ from scrapy.http import TextResponse, HtmlResponse, XmlResponse from scrapy.selector import Selector from scrapy.selector.lxmlsel import XmlXPathSelector, HtmlXPathSelector, XPathSelector from lxml import etree -from tests import mock class SelectorTestCase(unittest.TestCase): @@ -39,14 +38,21 @@ class SelectorTestCase(unittest.TestCase): self.assertEqual([x.extract() for x in sel.xpath("concat(//input[@name='a']/@value, //input[@name='b']/@value)")], [u'12']) - @mock.patch('scrapy.selector.unified.warnings') - def test_deprecated_root_argument(self, warnings): - root = etree.fromstring(u'') - sel = self.sscls(_root=root) - self.assertIs(root, sel.root) - warnings.warn.assert_called_once_with( - 'Argument `_root` is deprecated, use `root` instead', - mock.ANY, stacklevel=2) + def test_deprecated_root_argument(self): + with warnings.catch_warnings(record=True) as w: + root = etree.fromstring(u'') + sel = self.sscls(_root=root) + self.assertIs(root, sel.root) + self.assertEqual(str(w[-1].message), + 'Argument `_root` is deprecated, use `root` instead') + + def test_deprecated_root_argument_ambiguous(self): + with warnings.catch_warnings(record=True) as w: + _root = etree.fromstring(u'') + root = etree.fromstring(u'') + sel = self.sscls(_root=_root, root=root) + self.assertIs(root, sel.root) + self.assertIn('Ignoring deprecated `_root` argument', str(w[-1].message)) def test_representation_slice(self): body = u"

".format(50 * 'b') From 3a03ef7c08f0b4b454d13933ea2239b053824675 Mon Sep 17 00:00:00 2001 From: Elias Dorneles Date: Sun, 9 Aug 2015 15:23:43 -0300 Subject: [PATCH 13/17] cleanup tests for selectors and translators --- tests/py3-ignores.txt | 2 - tests/test_selector.py | 499 +-------------------------- tests/test_selector_csstranslator.py | 159 +-------- 3 files changed, 29 insertions(+), 631 deletions(-) diff --git a/tests/py3-ignores.txt b/tests/py3-ignores.txt index 8f5c0de48..5a009db36 100644 --- a/tests/py3-ignores.txt +++ b/tests/py3-ignores.txt @@ -25,8 +25,6 @@ tests/test_mail.py tests/test_pipeline_files.py tests/test_pipeline_images.py tests/test_proxy_connect.py -tests/test_selector_csstranslator.py -tests/test_selector.py tests/test_spidermiddleware_depth.py tests/test_spidermiddleware_httperror.py tests/test_spidermiddleware_offsite.py diff --git a/tests/test_selector.py b/tests/test_selector.py index dc37da86b..4806bb90b 100644 --- a/tests/test_selector.py +++ b/tests/test_selector.py @@ -12,18 +12,16 @@ from lxml import etree class SelectorTestCase(unittest.TestCase): - sscls = Selector - def test_simple_selection(self): """Simple selector tests""" - body = "

" - response = TextResponse(url="http://example.com", body=body) - sel = self.sscls(response) + body = u"

" + response = TextResponse(url="http://example.com", body=body, encoding='utf-8') + sel = Selector(response) xl = sel.xpath('//input') self.assertEqual(2, len(xl)) for x in xl: - assert isinstance(x, self.sscls) + assert isinstance(x, Selector) self.assertEqual(sel.xpath('//input').extract(), [x.extract() for x in sel.xpath('//input')]) @@ -41,7 +39,7 @@ class SelectorTestCase(unittest.TestCase): def test_deprecated_root_argument(self): with warnings.catch_warnings(record=True) as w: root = etree.fromstring(u'') - sel = self.sscls(_root=root) + sel = Selector(_root=root) self.assertIs(root, sel.root) self.assertEqual(str(w[-1].message), 'Argument `_root` is deprecated, use `root` instead') @@ -50,238 +48,22 @@ class SelectorTestCase(unittest.TestCase): with warnings.catch_warnings(record=True) as w: _root = etree.fromstring(u'') root = etree.fromstring(u'') - sel = self.sscls(_root=_root, root=root) + sel = Selector(_root=_root, root=root) self.assertIs(root, sel.root) self.assertIn('Ignoring deprecated `_root` argument', str(w[-1].message)) - def test_representation_slice(self): - body = u"

".format(50 * 'b') - response = TextResponse(url="http://example.com", body=body, encoding='utf8') - sel = self.sscls(response) - - self.assertEqual( - map(repr, sel.xpath('//input/@name')), - ["".format(40 * 'b')] - ) - - def test_representation_unicode_query(self): - body = u"

".format(50 * 'b') - response = TextResponse(url="http://example.com", body=body, encoding='utf8') - sel = self.sscls(response) - self.assertEqual( - map(repr, sel.xpath(u'//input[@value="\xa9"]/@value')), - [""] - ) - - def test_extract_first(self): - """Test if extract_first() returns first element""" - body = '
  • 1
  • 2
' - response = TextResponse(url="http://example.com", body=body) - sel = self.sscls(response) - - self.assertEqual(sel.xpath('//ul/li/text()').extract_first(), - sel.xpath('//ul/li/text()').extract()[0]) - - self.assertEqual(sel.xpath('//ul/li[@id="1"]/text()').extract_first(), - sel.xpath('//ul/li[@id="1"]/text()').extract()[0]) - - self.assertEqual(sel.xpath('//ul/li[2]/text()').extract_first(), - sel.xpath('//ul/li/text()').extract()[1]) - - self.assertEqual(sel.xpath('/ul/li[@id="doesnt-exist"]/text()').extract_first(), None) - - def test_extract_first_default(self): - """Test if extract_first() returns default value when no results found""" - body = '
  • 1
  • 2
' - response = TextResponse(url="http://example.com", body=body) - sel = self.sscls(response) - - self.assertEqual(sel.xpath('//div/text()').extract_first(default='missing'), 'missing') - - def test_re_first(self): - """Test if re_first() returns first matched element""" - body = '
  • 1
  • 2
' - response = TextResponse(url="http://example.com", body=body) - sel = self.sscls(response) - - self.assertEqual(sel.xpath('//ul/li/text()').re_first('\d'), - sel.xpath('//ul/li/text()').re('\d')[0]) - - self.assertEqual(sel.xpath('//ul/li[@id="1"]/text()').re_first('\d'), - sel.xpath('//ul/li[@id="1"]/text()').re('\d')[0]) - - self.assertEqual(sel.xpath('//ul/li[2]/text()').re_first('\d'), - sel.xpath('//ul/li/text()').re('\d')[1]) - - self.assertEqual(sel.xpath('/ul/li/text()').re_first('\w+'), None) - self.assertEqual(sel.xpath('/ul/li[@id="doesnt-exist"]/text()').re_first('\d'), None) - - def test_select_unicode_query(self): - body = u"

" - response = TextResponse(url="http://example.com", body=body, encoding='utf8') - sel = self.sscls(response) - self.assertEqual(sel.xpath(u'//input[@name="\xa9"]/@value').extract(), [u'1']) - - def test_list_elements_type(self): - """Test Selector returning the same type in selection methods""" - text = '

test

' - assert isinstance(self.sscls(text=text).xpath("//p")[0], self.sscls) - assert isinstance(self.sscls(text=text).css("p")[0], self.sscls) - - def test_boolean_result(self): - body = "

" - response = TextResponse(url="http://example.com", body=body) - xs = self.sscls(response) - self.assertEquals(xs.xpath("//input[@name='a']/@name='a'").extract(), [u'1']) - self.assertEquals(xs.xpath("//input[@name='a']/@name='n'").extract(), [u'0']) - - def test_differences_parsing_xml_vs_html(self): - """Test that XML and HTML Selector's behave differently""" - # some text which is parsed differently by XML and HTML flavors - text = '

Hello

' - hs = self.sscls(text=text, type='html') - self.assertEqual(hs.xpath("//div").extract(), - [u'

Hello

']) - - xs = self.sscls(text=text, type='xml') - self.assertEqual(xs.xpath("//div").extract(), - [u'

Hello

']) - def test_flavor_detection(self): - text = '

Hello

' - sel = self.sscls(XmlResponse('http://example.com', body=text)) + text = u'

Hello

' + sel = Selector(XmlResponse('http://example.com', body=text, encoding='utf-8')) self.assertEqual(sel.type, 'xml') self.assertEqual(sel.xpath("//div").extract(), [u'

Hello

']) - sel = self.sscls(HtmlResponse('http://example.com', body=text)) + sel = Selector(HtmlResponse('http://example.com', body=text, encoding='utf-8')) self.assertEqual(sel.type, 'html') self.assertEqual(sel.xpath("//div").extract(), [u'

Hello

']) - def test_nested_selectors(self): - """Nested selector tests""" - body = """ -
-
    -
  • one
  • two
    • -
-
-
-
    -
  • four
  • five
  • six
    • -
-
- """ - - response = HtmlResponse(url="http://example.com", body=body) - x = self.sscls(response) - divtwo = x.xpath('//div[@class="two"]') - self.assertEqual(divtwo.xpath("//li").extract(), - ["
  • one
    • ", "
  • two
    • ", "
  • four
    • ", "
  • five
    • ", "
  • six
    • "]) - self.assertEqual(divtwo.xpath("./ul/li").extract(), - ["
  • four
    • ", "
  • five
    • ", "
  • six
    • "]) - self.assertEqual(divtwo.xpath(".//li").extract(), - ["
  • four
    • ", "
  • five
    • ", "
  • six
    • "]) - self.assertEqual(divtwo.xpath("./li").extract(), []) - - def test_mixed_nested_selectors(self): - body = ''' -
    notme
    -

    text

    foo
    - ''' - sel = self.sscls(text=body) - self.assertEqual(sel.xpath('//div[@id="1"]').css('span::text').extract(), [u'me']) - self.assertEqual(sel.css('#1').xpath('./span/text()').extract(), [u'me']) - - def test_dont_strip(self): - sel = self.sscls(text='
    fff: zzz
    ') - self.assertEqual(sel.xpath("//text()").extract(), [u'fff: ', u'zzz']) - - def test_namespaces_simple(self): - body = """ - - take this - found - - """ - - response = XmlResponse(url="http://example.com", body=body) - x = self.sscls(response) - - x.register_namespace("somens", "http://scrapy.org") - self.assertEqual(x.xpath("//somens:a/text()").extract(), - [u'take this']) - - def test_namespaces_multiple(self): - body = """ - - hello - value - iron90Dried Rose - - """ - response = XmlResponse(url="http://example.com", body=body) - x = self.sscls(response) - x.register_namespace("xmlns", "http://webservices.amazon.com/AWSECommerceService/2005-10-05") - x.register_namespace("p", "http://www.scrapy.org/product") - x.register_namespace("b", "http://somens.com") - self.assertEqual(len(x.xpath("//xmlns:TestTag")), 1) - self.assertEqual(x.xpath("//b:Operation/text()").extract()[0], 'hello') - self.assertEqual(x.xpath("//xmlns:TestTag/@b:att").extract()[0], 'value') - self.assertEqual(x.xpath("//p:SecondTestTag/xmlns:price/text()").extract()[0], '90') - self.assertEqual(x.xpath("//p:SecondTestTag").xpath("./xmlns:price/text()")[0].extract(), '90') - self.assertEqual(x.xpath("//p:SecondTestTag/xmlns:material/text()").extract()[0], 'iron') - - def test_re(self): - body = """
    Name: Mary -
      -
    • Name: John
      • -
    • Age: 10
      • -
    • Name: Paul
      • -
    • Age: 20
      • -
    - Age: 20 -
    """ - response = HtmlResponse(url="http://example.com", body=body) - x = self.sscls(response) - - name_re = re.compile("Name: (\w+)") - self.assertEqual(x.xpath("//ul/li").re(name_re), - ["John", "Paul"]) - self.assertEqual(x.xpath("//ul/li").re("Age: (\d+)"), - ["10", "20"]) - - def test_re_intl(self): - body = """
    Evento: cumplea\xc3\xb1os
    """ - response = HtmlResponse(url="http://example.com", body=body, encoding='utf-8') - x = self.sscls(response) - self.assertEqual(x.xpath("//div").re("Evento: (\w+)"), [u'cumplea\xf1os']) - - def test_selector_over_text(self): - hs = self.sscls(text='lala') - self.assertEqual(hs.extract(), u'lala') - xs = self.sscls(text='lala', type='xml') - self.assertEqual(xs.extract(), u'lala') - self.assertEqual(xs.xpath('.').extract(), [u'lala']) - - def test_invalid_xpath(self): - "Test invalid xpath raises ValueError with the invalid xpath" - response = XmlResponse(url="http://example.com", body="") - x = self.sscls(response) - xpath = "//test[@foo='bar]" - self.assertRaisesRegexp(ValueError, re.escape(xpath), x.xpath, xpath) - - def test_invalid_xpath_unicode(self): - "Test *Unicode* invalid xpath raises ValueError with the invalid xpath" - response = XmlResponse(url="http://example.com", body="") - x = self.sscls(response) - xpath = u"//test[@foo='\u0431ar]" - encoded = xpath if six.PY3 else xpath.encode('unicode_escape') - self.assertRaisesRegexp(ValueError, re.escape(encoded), x.xpath, xpath) - def test_http_header_encoding_precedence(self): # u'\xa3' = pound symbol in unicode # u'\xc2\xa3' = pound symbol in utf-8 @@ -297,136 +79,28 @@ class SelectorTestCase(unittest.TestCase): headers = {'Content-Type': ['text/html; charset=utf-8']} response = HtmlResponse(url="http://example.com", headers=headers, body=html_utf8) - x = self.sscls(response) + x = Selector(response) self.assertEquals(x.xpath("//span[@id='blank']/text()").extract(), [u'\xa3']) - def test_empty_bodies(self): - # shouldn't raise errors - r1 = TextResponse('http://www.example.com', body='') - self.sscls(r1).xpath('//text()').extract() - - def test_null_bytes(self): - # shouldn't raise errors - r1 = TextResponse('http://www.example.com', \ - body='pre\x00post', \ - encoding='utf-8') - self.sscls(r1).xpath('//text()').extract() - def test_badly_encoded_body(self): # \xe9 alone isn't valid utf8 sequence r1 = TextResponse('http://www.example.com', \ - body='

    an Jos\xe9 de

    ', \ + body=u'

    an Jos\xe9 de

    ', \ encoding='utf-8') - self.sscls(r1).xpath('//text()').extract() - - def test_select_on_unevaluable_nodes(self): - r = self.sscls(text=u'some text') - # Text node - x1 = r.xpath('//text()') - self.assertEquals(x1.extract(), [u'some text']) - self.assertEquals(x1.xpath('.//b').extract(), []) - # Tag attribute - x1 = r.xpath('//span/@class') - self.assertEquals(x1.extract(), [u'big']) - self.assertEquals(x1.xpath('.//text()').extract(), []) - - def test_select_on_text_nodes(self): - r = self.sscls(text=u'
    Options:opt1
    Otheropt2
    ') - x1 = r.xpath("//div/descendant::text()[preceding-sibling::b[contains(text(), 'Options')]]") - self.assertEquals(x1.extract(), [u'opt1']) - - x1 = r.xpath("//div/descendant::text()/preceding-sibling::b[contains(text(), 'Options')]") - self.assertEquals(x1.extract(), [u'Options:']) - - def test_nested_select_on_text_nodes(self): - # FIXME: does not work with lxml backend [upstream] - r = self.sscls(text=u'
    Options:opt1
    Otheropt2
    ') - x1 = r.xpath("//div/descendant::text()") - x2 = x1.xpath("./preceding-sibling::b[contains(text(), 'Options')]") - self.assertEquals(x2.extract(), [u'Options:']) - test_nested_select_on_text_nodes.skip = "Text nodes lost parent node reference in lxml" + Selector(r1).xpath('//text()').extract() def test_weakref_slots(self): """Check that classes are using slots and are weak-referenceable""" - x = self.sscls(text='') + x = Selector(text='') weakref.ref(x) assert not hasattr(x, '__dict__'), "%s does not use __slots__" % \ x.__class__.__name__ - def test_remove_namespaces(self): - xml = """ - - - - -""" - sel = self.sscls(XmlResponse("http://example.com/feed.atom", body=xml)) - self.assertEqual(len(sel.xpath("//link")), 0) - sel.remove_namespaces() - self.assertEqual(len(sel.xpath("//link")), 2) - - def test_remove_attributes_namespaces(self): - xml = """ - - - - -""" - sel = self.sscls(XmlResponse("http://example.com/feed.atom", body=xml)) - self.assertEqual(len(sel.xpath("//link/@type")), 0) - sel.remove_namespaces() - self.assertEqual(len(sel.xpath("//link/@type")), 2) - - def test_smart_strings(self): - """Lxml smart strings return values""" - - class SmartStringsSelector(Selector): - _lxml_smart_strings = True - - body = """ -
    -
      -
    • one
    • two
      • -
    -
    -
    -
      -
    • four
    • five
    • six
      • -
    -
    - """ - - response = HtmlResponse(url="http://example.com", body=body) - - # .getparent() is available for text nodes and attributes - # only when smart_strings are on - x = self.sscls(response) - li_text = x.xpath('//li/text()') - self.assertFalse(any(map(lambda e: hasattr(e._root, 'getparent'), li_text))) - div_class = x.xpath('//div/@class') - self.assertFalse(any(map(lambda e: hasattr(e._root, 'getparent'), div_class))) - - x = SmartStringsSelector(response) - li_text = x.xpath('//li/text()') - self.assertTrue(all(map(lambda e: hasattr(e._root, 'getparent'), li_text))) - div_class = x.xpath('//div/@class') - self.assertTrue(all(map(lambda e: hasattr(e._root, 'getparent'), div_class))) - - def test_xml_entity_expansion(self): - malicious_xml = ''\ - ' ]>&xxe;' - - response = XmlResponse('http://example.com', body=malicious_xml) - sel = self.sscls(response=response) - - self.assertEqual(sel.extract(), '&xxe;') - class DeprecatedXpathSelectorTest(unittest.TestCase): - text = '

    Hello

    ' + text = u'

    Hello

    ' def test_warnings_xpathselector(self): cls = XPathSelector @@ -504,147 +178,4 @@ class DeprecatedXpathSelectorTest(unittest.TestCase): self.assertTrue(isinstance(sel, Selector)) self.assertTrue(isinstance(usel, Selector)) self.assertTrue(isinstance(sel, XPathSelector)) - self.assertTrue(isinstance(usel, XPathSelector)) - - def test_xpathselector(self): - with warnings.catch_warnings(): - warnings.simplefilter('ignore', ScrapyDeprecationWarning) - hs = XPathSelector(text=self.text) - self.assertEqual(hs.select("//div").extract(), - [u'

    Hello

    ']) - self.assertRaises(RuntimeError, hs.css, 'div') - - def test_htmlxpathselector(self): - with warnings.catch_warnings(): - warnings.simplefilter('ignore', ScrapyDeprecationWarning) - hs = HtmlXPathSelector(text=self.text) - self.assertEqual(hs.select("//div").extract(), - [u'

    Hello

    ']) - self.assertRaises(RuntimeError, hs.css, 'div') - - def test_xmlxpathselector(self): - with warnings.catch_warnings(): - warnings.simplefilter('ignore', ScrapyDeprecationWarning) - xs = XmlXPathSelector(text=self.text) - self.assertEqual(xs.select("//div").extract(), - [u'

    Hello

    ']) - self.assertRaises(RuntimeError, xs.css, 'div') - - -class ExsltTestCase(unittest.TestCase): - - sscls = Selector - - def test_regexp(self): - """EXSLT regular expression tests""" - body = """ -

    -
    - first link - second link - EXSLT match example -
    - """ - response = TextResponse(url="http://example.com", body=body) - sel = self.sscls(response) - - # re:test() - self.assertEqual( - sel.xpath( - '//input[re:test(@name, "[A-Z]+", "i")]').extract(), - [x.extract() for x in sel.xpath('//input[re:test(@name, "[A-Z]+", "i")]')]) - self.assertEqual( - [x.extract() - for x in sel.xpath( - '//a[re:test(@href, "\.html$")]/text()')], - [u'first link', u'second link']) - self.assertEqual( - [x.extract() - for x in sel.xpath( - '//a[re:test(@href, "first")]/text()')], - [u'first link']) - self.assertEqual( - [x.extract() - for x in sel.xpath( - '//a[re:test(@href, "second")]/text()')], - [u'second link']) - - - # re:match() is rather special: it returns a node-set of nodes - #[u'http://www.bayes.co.uk/xml/index.xml?/xml/utils/rechecker.xml', - #u'http', - #u'www.bayes.co.uk', - #u'', - #u'/xml/index.xml?/xml/utils/rechecker.xml'] - self.assertEqual( - sel.xpath('re:match(//a[re:test(@href, "\.xml$")]/@href,' - '"(\w+):\/\/([^/:]+)(:\d*)?([^# ]*)")/text()').extract(), - [u'http://www.bayes.co.uk/xml/index.xml?/xml/utils/rechecker.xml', - u'http', - u'www.bayes.co.uk', - u'', - u'/xml/index.xml?/xml/utils/rechecker.xml']) - - - - # re:replace() - self.assertEqual( - sel.xpath('re:replace(//a[re:test(@href, "\.xml$")]/@href,' - '"(\w+)://(.+)(\.xml)", "","https://\\2.html")').extract(), - [u'https://www.bayes.co.uk/xml/index.xml?/xml/utils/rechecker.html']) - - def test_set(self): - """EXSLT set manipulation tests""" - # microdata example from http://schema.org/Event - body=""" -
    - - - - Thu, 04/21/16 - 8:00 p.m. - -
    - -
    - Philadelphia, - PA -
    -
    - -
    - Priced from: $35 - 1938 tickets left -
    -
    - """ - response = TextResponse(url="http://example.com", body=body) - sel = self.sscls(response) - - self.assertEqual( - sel.xpath('''//div[@itemtype="http://schema.org/Event"] - //@itemprop''').extract(), - [u'url', - u'name', - u'startDate', - u'location', - u'url', - u'address', - u'addressLocality', - u'addressRegion', - u'offers', - u'lowPrice', - u'offerCount'] - ) - - self.assertEqual(sel.xpath(''' - set:difference(//div[@itemtype="http://schema.org/Event"] - //@itemprop, - //div[@itemtype="http://schema.org/Event"] - //*[@itemscope]/*/@itemprop)''').extract(), - [u'url', u'name', u'startDate', u'location', u'offers']) + self.assertTrue(isinstance(usel, XPathSelector)) \ No newline at end of file diff --git a/tests/test_selector_csstranslator.py b/tests/test_selector_csstranslator.py index 1bc8882f8..2d82fcba7 100644 --- a/tests/test_selector_csstranslator.py +++ b/tests/test_selector_csstranslator.py @@ -1,153 +1,22 @@ """ Selector tests for cssselect backend """ +import warnings from twisted.trial import unittest -from scrapy.http import HtmlResponse -from scrapy.selector.csstranslator import ScrapyHTMLTranslator -from scrapy.selector import Selector -from cssselect.parser import SelectorSyntaxError -from cssselect.xpath import ExpressionError +from scrapy.selector.csstranslator import ( + ScrapyHTMLTranslator, + ScrapyGenericTranslator, + ScrapyXPathExpr +) -HTMLBODY = b''' - - -
    - - - link -

    - lorem ipsum text - hi there - guy - - - - - - - -

    - - -
    -

    - - - - -
    - - -''' +class DeprecatedClassesTest(unittest.TestCase): + + def test_deprecated_warnings(self): + for cls in [ScrapyHTMLTranslator, ScrapyGenericTranslator, ScrapyXPathExpr]: + with warnings.catch_warnings(record=True) as w: + obj = cls() + self.assertIn('%s is deprecated' % cls.__name__, str(w[-1].message), + 'Missing deprecate warning for %s' % cls.__name__) -class TranslatorMixinTest(unittest.TestCase): - - tr_cls = ScrapyHTMLTranslator - - def setUp(self): - self.tr = self.tr_cls() - self.c2x = self.tr.css_to_xpath - - def test_attr_function(self): - cases = [ - ('::attr(name)', u'descendant-or-self::*/@name'), - ('a::attr(href)', u'descendant-or-self::a/@href'), - ('a ::attr(img)', u'descendant-or-self::a/descendant-or-self::*/@img'), - ('a > ::attr(class)', u'descendant-or-self::a/*/@class'), - ] - for css, xpath in cases: - self.assertEqual(self.c2x(css), xpath, css) - - def test_attr_function_exception(self): - cases = [ - ('::attr(12)', ExpressionError), - ('::attr(34test)', ExpressionError), - ('::attr(@href)', SelectorSyntaxError), - ] - for css, exc in cases: - self.assertRaises(exc, self.c2x, css) - - def test_text_pseudo_element(self): - cases = [ - ('::text', u'descendant-or-self::text()'), - ('p::text', u'descendant-or-self::p/text()'), - ('p ::text', u'descendant-or-self::p/descendant-or-self::text()'), - ('#id::text', u"descendant-or-self::*[@id = 'id']/text()"), - ('p#id::text', u"descendant-or-self::p[@id = 'id']/text()"), - ('p#id ::text', u"descendant-or-self::p[@id = 'id']/descendant-or-self::text()"), - ('p#id > ::text', u"descendant-or-self::p[@id = 'id']/*/text()"), - ('p#id ~ ::text', u"descendant-or-self::p[@id = 'id']/following-sibling::*/text()"), - ('a[href]::text', u'descendant-or-self::a[@href]/text()'), - ('a[href] ::text', u'descendant-or-self::a[@href]/descendant-or-self::text()'), - ('p::text, a::text', u"descendant-or-self::p/text() | descendant-or-self::a/text()"), - ] - for css, xpath in cases: - self.assertEqual(self.c2x(css), xpath, css) - - def test_pseudo_function_exception(self): - cases = [ - ('::attribute(12)', ExpressionError), - ('::text()', ExpressionError), - ('::attr(@href)', SelectorSyntaxError), - ] - for css, exc in cases: - self.assertRaises(exc, self.c2x, css) - - def test_unknown_pseudo_element(self): - cases = [ - ('::text-node', ExpressionError), - ] - for css, exc in cases: - self.assertRaises(exc, self.c2x, css) - - def test_unknown_pseudo_class(self): - cases = [ - (':text', ExpressionError), - (':attribute(name)', ExpressionError), - ] - for css, exc in cases: - self.assertRaises(exc, self.c2x, css) - - -class CSSSelectorTest(unittest.TestCase): - - sscls = Selector - - def setUp(self): - self.htmlresponse = HtmlResponse('http://example.com', body=HTMLBODY) - self.sel = self.sscls(self.htmlresponse) - - def x(self, *a, **kw): - return [v.strip() for v in self.sel.css(*a, **kw).extract() if v.strip()] - - def test_selector_simple(self): - for x in self.sel.css('input'): - self.assertTrue(isinstance(x, self.sel.__class__), x) - self.assertEqual(self.sel.css('input').extract(), - [x.extract() for x in self.sel.css('input')]) - - def test_text_pseudo_element(self): - self.assertEqual(self.x('#p-b2'), [u'guy']) - self.assertEqual(self.x('#p-b2::text'), [u'guy']) - self.assertEqual(self.x('#p-b2 ::text'), [u'guy']) - self.assertEqual(self.x('#paragraph::text'), [u'lorem ipsum text']) - self.assertEqual(self.x('#paragraph ::text'), [u'lorem ipsum text', u'hi', u'there', u'guy']) - self.assertEqual(self.x('p::text'), [u'lorem ipsum text']) - self.assertEqual(self.x('p ::text'), [u'lorem ipsum text', u'hi', u'there', u'guy']) - - def test_attribute_function(self): - self.assertEqual(self.x('#p-b2::attr(id)'), [u'p-b2']) - self.assertEqual(self.x('.cool-footer::attr(class)'), [u'cool-footer']) - self.assertEqual(self.x('.cool-footer ::attr(id)'), [u'foobar-div', u'foobar-span']) - self.assertEqual(self.x('map[name="dummymap"] ::attr(shape)'), [u'circle', u'default']) - - def test_nested_selector(self): - self.assertEqual(self.sel.css('p').css('b::text').extract(), - [u'hi', u'guy']) - self.assertEqual(self.sel.css('div').css('area:last-child').extract(), - [u'']) From 8ef5aa2ffce4b2b8955b58b532920098f6fe8ea4 Mon Sep 17 00:00:00 2001 From: Elias Dorneles Date: Tue, 11 Aug 2015 13:58:53 -0300 Subject: [PATCH 14/17] using bytes for response body in tests --- tests/test_selector.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/tests/test_selector.py b/tests/test_selector.py index 4806bb90b..2d6d8c439 100644 --- a/tests/test_selector.py +++ b/tests/test_selector.py @@ -1,9 +1,6 @@ -import re import warnings import weakref -import six from twisted.trial import unittest -from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import TextResponse, HtmlResponse, XmlResponse from scrapy.selector import Selector from scrapy.selector.lxmlsel import XmlXPathSelector, HtmlXPathSelector, XPathSelector @@ -14,7 +11,7 @@ class SelectorTestCase(unittest.TestCase): def test_simple_selection(self): """Simple selector tests""" - body = u"

    " + body = b"

    " response = TextResponse(url="http://example.com", body=body, encoding='utf-8') sel = Selector(response) @@ -53,7 +50,7 @@ class SelectorTestCase(unittest.TestCase): self.assertIn('Ignoring deprecated `_root` argument', str(w[-1].message)) def test_flavor_detection(self): - text = u'

    Hello

    ' + text = b'

    Hello

    ' sel = Selector(XmlResponse('http://example.com', body=text, encoding='utf-8')) self.assertEqual(sel.type, 'xml') self.assertEqual(sel.xpath("//div").extract(), @@ -86,7 +83,7 @@ class SelectorTestCase(unittest.TestCase): def test_badly_encoded_body(self): # \xe9 alone isn't valid utf8 sequence r1 = TextResponse('http://www.example.com', \ - body=u'

    an Jos\xe9 de

    ', \ + body=b'

    an Jos\xe9 de

    ', \ encoding='utf-8') Selector(r1).xpath('//text()').extract() @@ -100,7 +97,7 @@ class SelectorTestCase(unittest.TestCase): class DeprecatedXpathSelectorTest(unittest.TestCase): - text = u'

    Hello

    ' + text = '

    Hello

    ' def test_warnings_xpathselector(self): cls = XPathSelector @@ -178,4 +175,4 @@ class DeprecatedXpathSelectorTest(unittest.TestCase): self.assertTrue(isinstance(sel, Selector)) self.assertTrue(isinstance(usel, Selector)) self.assertTrue(isinstance(sel, XPathSelector)) - self.assertTrue(isinstance(usel, XPathSelector)) \ No newline at end of file + self.assertTrue(isinstance(usel, XPathSelector)) From e50610bd3a8c5a5bad3f19353ce5214f11333e74 Mon Sep 17 00:00:00 2001 From: Elias Dorneles Date: Tue, 11 Aug 2015 14:06:24 -0300 Subject: [PATCH 15/17] set base_url in kwargs to be fully backward compatible --- scrapy/selector/unified.py | 1 + tests/test_selector.py | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/scrapy/selector/unified.py b/scrapy/selector/unified.py index a8f80c84d..25cf6c98d 100644 --- a/scrapy/selector/unified.py +++ b/scrapy/selector/unified.py @@ -46,6 +46,7 @@ class Selector(ParselSelector, object_ref): if response is not None: text = response.body_as_unicode() + kwargs.setdefault('base_url', response.url) self.response = response super(Selector, self).__init__(text=text, type=st, root=root, **kwargs) diff --git a/tests/test_selector.py b/tests/test_selector.py index 2d6d8c439..19b807a3f 100644 --- a/tests/test_selector.py +++ b/tests/test_selector.py @@ -33,6 +33,13 @@ class SelectorTestCase(unittest.TestCase): self.assertEqual([x.extract() for x in sel.xpath("concat(//input[@name='a']/@value, //input[@name='b']/@value)")], [u'12']) + def test_root_base_url(self): + body = b'' + url = "http://example.com" + response = TextResponse(url=url, body=body, encoding='utf-8') + sel = Selector(response) + self.assertEqual(url, sel.root.base) + def test_deprecated_root_argument(self): with warnings.catch_warnings(record=True) as w: root = etree.fromstring(u'') From 766c2551527574936d3210188c9b95b3f8ea726a Mon Sep 17 00:00:00 2001 From: Elias Dorneles Date: Tue, 11 Aug 2015 15:20:33 -0300 Subject: [PATCH 16/17] upgrade parsel and add shim for deprecated selectorlist methods --- requirements.txt | 2 +- scrapy/selector/unified.py | 17 ++++++++++++++++- tests/test_selector.py | 22 ++++++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/requirements.txt b/requirements.txt index afefb9d33..368e9340b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,4 +7,4 @@ queuelib six>=1.5.2 PyDispatcher>=2.0.5 service_identity -parsel>=0.9.3 +parsel>=0.9.5 diff --git a/scrapy/selector/unified.py b/scrapy/selector/unified.py index 25cf6c98d..dddf80c06 100644 --- a/scrapy/selector/unified.py +++ b/scrapy/selector/unified.py @@ -3,7 +3,7 @@ XPath selectors based on lxml """ import warnings -from parsel import Selector as ParselSelector, SelectorList +from parsel import Selector as ParselSelector, SelectorList as ParselSelectorList from scrapy.utils.trackref import object_ref from scrapy.utils.python import to_bytes from scrapy.http import HtmlResponse, XmlResponse @@ -26,9 +26,24 @@ def _response_from_text(text, st): body=to_bytes(text, 'utf-8')) +class SelectorList(ParselSelectorList, object_ref): + @deprecated(use_instead='.extract()') + def extract_unquoted(self): + return [x.extract_unquoted() for x in self] + + @deprecated(use_instead='.xpath()') + def x(self, xpath): + return self.select(xpath) + + @deprecated(use_instead='.xpath()') + def select(self, xpath): + return self.xpath(xpath) + + class Selector(ParselSelector, object_ref): __slots__ = ['response'] + selectorlist_cls = SelectorList def __init__(self, response=None, text=None, type=None, root=None, _root=None, **kwargs): st = _st(response, type or self._default_type) diff --git a/tests/test_selector.py b/tests/test_selector.py index 19b807a3f..141455b66 100644 --- a/tests/test_selector.py +++ b/tests/test_selector.py @@ -101,6 +101,28 @@ class SelectorTestCase(unittest.TestCase): assert not hasattr(x, '__dict__'), "%s does not use __slots__" % \ x.__class__.__name__ + def test_deprecated_selector_methods(self): + sel = Selector(TextResponse(url="http://example.com", body=b'

    some text

    ')) + + with warnings.catch_warnings(record=True) as w: + sel.select('//p') + self.assertSubstring('Use .xpath() instead', str(w[-1].message)) + + with warnings.catch_warnings(record=True) as w: + sel.extract_unquoted() + self.assertSubstring('Use .extract() instead', str(w[-1].message)) + + def test_deprecated_selectorlist_methods(self): + sel = Selector(TextResponse(url="http://example.com", body=b'

    some text

    ')) + + with warnings.catch_warnings(record=True) as w: + sel.xpath('//p').select('.') + self.assertSubstring('Use .xpath() instead', str(w[-1].message)) + + with warnings.catch_warnings(record=True) as w: + sel.xpath('//p').extract_unquoted() + self.assertSubstring('Use .extract() instead', str(w[-1].message)) + class DeprecatedXpathSelectorTest(unittest.TestCase): From a5abd19e846521b19886b4e7e2ac82f0c401b088 Mon Sep 17 00:00:00 2001 From: Elias Dorneles Date: Tue, 11 Aug 2015 15:58:29 -0300 Subject: [PATCH 17/17] make Parsel's Selector more private, remove direct dependency of ParselSelectorList --- scrapy/selector/unified.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scrapy/selector/unified.py b/scrapy/selector/unified.py index dddf80c06..5d77f7624 100644 --- a/scrapy/selector/unified.py +++ b/scrapy/selector/unified.py @@ -3,7 +3,7 @@ XPath selectors based on lxml """ import warnings -from parsel import Selector as ParselSelector, SelectorList as ParselSelectorList +from parsel import Selector as _ParselSelector from scrapy.utils.trackref import object_ref from scrapy.utils.python import to_bytes from scrapy.http import HtmlResponse, XmlResponse @@ -26,7 +26,7 @@ def _response_from_text(text, st): body=to_bytes(text, 'utf-8')) -class SelectorList(ParselSelectorList, object_ref): +class SelectorList(_ParselSelector.selectorlist_cls, object_ref): @deprecated(use_instead='.extract()') def extract_unquoted(self): return [x.extract_unquoted() for x in self] @@ -40,7 +40,7 @@ class SelectorList(ParselSelectorList, object_ref): return self.xpath(xpath) -class Selector(ParselSelector, object_ref): +class Selector(_ParselSelector, object_ref): __slots__ = ['response'] selectorlist_cls = SelectorList