diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index 1bf5874a0..8c4049f85 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -402,10 +402,12 @@ Crawling rules of links extracted from each response using the specified ``link_extractor``. This is mainly used for filtering purposes. - ``process_request`` is a callable, or a string (in which case a method from - the spider object with that name will be used) which will be called with - every request extracted by this rule, and must return a request or None (to - filter out the request). + ``process_request`` is a callable (or a string, in which case a method from + the spider object with that name will be used) which will be called for every + :class:`~scrapy.http.Request` extracted by this rule. This callable should + take said request as first argument and the :class:`~scrapy.http.Response` + from which the request originated as second argument. It must return a + ``Request`` object or ``None`` (to filter out the request). CrawlSpider example ~~~~~~~~~~~~~~~~~~~ diff --git a/scrapy/spiders/crawl.py b/scrapy/spiders/crawl.py index e5ac72e18..6db3a1e06 100644 --- a/scrapy/spiders/crawl.py +++ b/scrapy/spiders/crawl.py @@ -6,29 +6,55 @@ See documentation in docs/topics/spiders.rst """ import copy +import warnings + import six +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Request, HtmlResponse from scrapy.utils.spider import iterate_spider_output +from scrapy.utils.python import get_func_args from scrapy.spiders import Spider -def identity(x): - return x +def _identity(request, response): + return request + + +def _get_method(method, spider): + if callable(method): + return method + elif isinstance(method, six.string_types): + return getattr(spider, method, None) class Rule(object): - def __init__(self, link_extractor, callback=None, cb_kwargs=None, follow=None, process_links=None, process_request=identity): + def __init__(self, link_extractor, callback=None, cb_kwargs=None, follow=None, process_links=None, process_request=None): self.link_extractor = link_extractor self.callback = callback self.cb_kwargs = cb_kwargs or {} self.process_links = process_links - self.process_request = process_request - if follow is None: - self.follow = False if callback else True - else: - self.follow = follow + self.process_request = process_request or _identity + self.process_request_argcount = None + self.follow = follow if follow is not None else not callback + + def _compile(self, spider): + self.callback = _get_method(self.callback, spider) + self.process_links = _get_method(self.process_links, spider) + self.process_request = _get_method(self.process_request, spider) + self.process_request_argcount = len(get_func_args(self.process_request)) + if self.process_request_argcount == 1: + msg = 'Rule.process_request should accept two arguments (request, response), accepting only one is deprecated' + warnings.warn(msg, category=ScrapyDeprecationWarning, stacklevel=2) + + def _process_request(self, request, response): + """ + Wrapper around the request processing function to maintain backward + compatibility with functions that do not take a Response object + """ + args = [request] if self.process_request_argcount == 1 else [request, response] + return self.process_request(*args) class CrawlSpider(Spider): @@ -64,8 +90,8 @@ class CrawlSpider(Spider): links = rule.process_links(links) for link in links: seen.add(link) - r = self._build_request(n, link) - yield rule.process_request(r) + request = self._build_request(n, link) + yield rule._process_request(request, response) def _response_downloaded(self, response): rule = self._rules[response.meta['rule']] @@ -83,17 +109,9 @@ class CrawlSpider(Spider): yield request_or_item def _compile_rules(self): - def get_method(method): - if callable(method): - return method - elif isinstance(method, six.string_types): - return getattr(self, method, None) - self._rules = [copy.copy(r) for r in self.rules] for rule in self._rules: - rule.callback = get_method(rule.callback) - rule.process_links = get_method(rule.process_links) - rule.process_request = get_method(rule.process_request) + rule._compile(self) @classmethod def from_crawler(cls, crawler, *args, **kwargs): diff --git a/tests/test_spider.py b/tests/test_spider.py index fefdaa403..83fb68c2f 100644 --- a/tests/test_spider.py +++ b/tests/test_spider.py @@ -105,11 +105,11 @@ class SpiderTest(unittest.TestCase): def test_logger(self): spider = self.spider_class('example.com') - with LogCapture() as l: + with LogCapture() as lc: spider.logger.info('test log msg') - l.check(('example.com', 'INFO', 'test log msg')) + lc.check(('example.com', 'INFO', 'test log msg')) - record = l.records[0] + record = lc.records[0] self.assertIn('spider', record.__dict__) self.assertIs(record.spider, spider) @@ -190,12 +190,11 @@ class CrawlSpiderTest(SpiderTest): def test_process_links(self): - response = HtmlResponse("http://example.org/somepage/index.html", - body=self.test_body) + response = HtmlResponse("http://example.org/somepage/index.html", body=self.test_body) class _CrawlSpider(self.spider_class): - name="test" - allowed_domains=['example.org'] + name = "test" + allowed_domains = ['example.org'] rules = ( Rule(LinkExtractor(), process_links="dummy_process_links"), ) @@ -208,24 +207,24 @@ class CrawlSpiderTest(SpiderTest): self.assertEqual(len(output), 3) self.assertTrue(all(map(lambda r: isinstance(r, Request), output))) self.assertEqual([r.url for r in output], - ['http://example.org/somepage/item/12.html', - 'http://example.org/about.html', - 'http://example.org/nofollow.html']) + ['http://example.org/somepage/item/12.html', + 'http://example.org/about.html', + 'http://example.org/nofollow.html']) def test_process_links_filter(self): - response = HtmlResponse("http://example.org/somepage/index.html", - body=self.test_body) + response = HtmlResponse("http://example.org/somepage/index.html", body=self.test_body) class _CrawlSpider(self.spider_class): import re - name="test" - allowed_domains=['example.org'] + name = "test" + allowed_domains = ['example.org'] rules = ( Rule(LinkExtractor(), process_links="filter_process_links"), ) _test_regex = re.compile('nofollow') + def filter_process_links(self, links): return [link for link in links if not self._test_regex.search(link.url)] @@ -235,17 +234,16 @@ class CrawlSpiderTest(SpiderTest): self.assertEqual(len(output), 2) self.assertTrue(all(map(lambda r: isinstance(r, Request), output))) self.assertEqual([r.url for r in output], - ['http://example.org/somepage/item/12.html', - 'http://example.org/about.html']) + ['http://example.org/somepage/item/12.html', + 'http://example.org/about.html']) def test_process_links_generator(self): - response = HtmlResponse("http://example.org/somepage/index.html", - body=self.test_body) + response = HtmlResponse("http://example.org/somepage/index.html", body=self.test_body) class _CrawlSpider(self.spider_class): - name="test" - allowed_domains=['example.org'] + name = "test" + allowed_domains = ['example.org'] rules = ( Rule(LinkExtractor(), process_links="dummy_process_links"), ) @@ -259,9 +257,113 @@ class CrawlSpiderTest(SpiderTest): self.assertEqual(len(output), 3) self.assertTrue(all(map(lambda r: isinstance(r, Request), output))) self.assertEqual([r.url for r in output], - ['http://example.org/somepage/item/12.html', - 'http://example.org/about.html', - 'http://example.org/nofollow.html']) + ['http://example.org/somepage/item/12.html', + 'http://example.org/about.html', + 'http://example.org/nofollow.html']) + + def test_process_request(self): + + response = HtmlResponse("http://example.org/somepage/index.html", body=self.test_body) + + def process_request_change_domain(request): + return request.replace(url=request.url.replace('.org', '.com')) + + class _CrawlSpider(self.spider_class): + name = "test" + allowed_domains = ['example.org'] + rules = ( + Rule(LinkExtractor(), process_request=process_request_change_domain), + ) + + with warnings.catch_warnings(record=True) as cw: + spider = _CrawlSpider() + output = list(spider._requests_to_follow(response)) + self.assertEqual(len(output), 3) + self.assertTrue(all(map(lambda r: isinstance(r, Request), output))) + self.assertEqual([r.url for r in output], + ['http://example.com/somepage/item/12.html', + 'http://example.com/about.html', + 'http://example.com/nofollow.html']) + self.assertEqual(len(cw), 1) + self.assertEqual(cw[0].category, ScrapyDeprecationWarning) + + def test_process_request_with_response(self): + + response = HtmlResponse("http://example.org/somepage/index.html", body=self.test_body) + + def process_request_meta_response_class(request, response): + request.meta['response_class'] = response.__class__.__name__ + return request + + class _CrawlSpider(self.spider_class): + name = "test" + allowed_domains = ['example.org'] + rules = ( + Rule(LinkExtractor(), process_request=process_request_meta_response_class), + ) + + spider = _CrawlSpider() + output = list(spider._requests_to_follow(response)) + self.assertEqual(len(output), 3) + self.assertTrue(all(map(lambda r: isinstance(r, Request), output))) + self.assertEqual([r.url for r in output], + ['http://example.org/somepage/item/12.html', + 'http://example.org/about.html', + 'http://example.org/nofollow.html']) + self.assertEqual([r.meta['response_class'] for r in output], + ['HtmlResponse', 'HtmlResponse', 'HtmlResponse']) + + def test_process_request_instance_method(self): + + response = HtmlResponse("http://example.org/somepage/index.html", body=self.test_body) + + class _CrawlSpider(self.spider_class): + name = "test" + allowed_domains = ['example.org'] + rules = ( + Rule(LinkExtractor(), process_request='process_request_upper'), + ) + + def process_request_upper(self, request): + return request.replace(url=request.url.upper()) + + with warnings.catch_warnings(record=True) as cw: + spider = _CrawlSpider() + output = list(spider._requests_to_follow(response)) + self.assertEqual(len(output), 3) + self.assertTrue(all(map(lambda r: isinstance(r, Request), output))) + self.assertEqual([r.url for r in output], + ['http://EXAMPLE.ORG/SOMEPAGE/ITEM/12.HTML', + 'http://EXAMPLE.ORG/ABOUT.HTML', + 'http://EXAMPLE.ORG/NOFOLLOW.HTML']) + self.assertEqual(len(cw), 1) + self.assertEqual(cw[0].category, ScrapyDeprecationWarning) + + def test_process_request_instance_method_with_response(self): + + response = HtmlResponse("http://example.org/somepage/index.html", body=self.test_body) + + class _CrawlSpider(self.spider_class): + name = "test" + allowed_domains = ['example.org'] + rules = ( + Rule(LinkExtractor(), process_request='process_request_meta_response_class'), + ) + + def process_request_meta_response_class(self, request, response): + request.meta['response_class'] = response.__class__.__name__ + return request + + spider = _CrawlSpider() + output = list(spider._requests_to_follow(response)) + self.assertEqual(len(output), 3) + self.assertTrue(all(map(lambda r: isinstance(r, Request), output))) + self.assertEqual([r.url for r in output], + ['http://example.org/somepage/item/12.html', + 'http://example.org/about.html', + 'http://example.org/nofollow.html']) + self.assertEqual([r.meta['response_class'] for r in output], + ['HtmlResponse', 'HtmlResponse', 'HtmlResponse']) def test_follow_links_attribute_population(self): crawler = get_crawler()