diff --git a/docs/faq.rst b/docs/faq.rst index 657802fd3..6294dd52b 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -138,39 +138,37 @@ See previous question. How can I prevent memory errors due to many allowed domains? ------------------------------------------------------------ -If you have a spider with a long list of -:attr:`~scrapy.Spider.allowed_domains` (e.g. 50,000+), consider -replacing the default -:class:`~scrapy.spidermiddlewares.offsite.OffsiteMiddleware` spider middleware -with a :ref:`custom spider middleware ` that requires -less memory. For example: +If you have a spider with a long list of :attr:`~scrapy.Spider.allowed_domains` +(e.g. 50,000+), consider replacing the default +:class:`~scrapy.downloadermiddlewares.offsite.OffsiteMiddleware` downloader +middleware with a :ref:`custom downloader middleware +` that requires less memory. For example: - If your domain names are similar enough, use your own regular expression - instead joining the strings in - :attr:`~scrapy.Spider.allowed_domains` into a complex regular - expression. + instead joining the strings in :attr:`~scrapy.Spider.allowed_domains` into + a complex regular expression. - If you can `meet the installation requirements`_, use pyre2_ instead of Python’s re_ to compile your URL-filtering regular expression. See :issue:`1908`. -See also other suggestions at `StackOverflow`_. +See also `other suggestions at StackOverflow +`__. .. note:: Remember to disable - :class:`scrapy.spidermiddlewares.offsite.OffsiteMiddleware` when you enable - your custom implementation: + :class:`scrapy.downloadermiddlewares.offsite.OffsiteMiddleware` when you + enable your custom implementation: .. code-block:: python - SPIDER_MIDDLEWARES = { - "scrapy.spidermiddlewares.offsite.OffsiteMiddleware": None, - "myproject.middlewares.CustomOffsiteMiddleware": 500, + DOWNLOADER_MIDDLEWARES = { + "scrapy.downloadermiddlewares.offsite.OffsiteMiddleware": None, + "myproject.middlewares.CustomOffsiteMiddleware": 50, } .. _meet the installation requirements: https://github.com/andreasvc/pyre2#installation .. _pyre2: https://github.com/andreasvc/pyre2 .. _re: https://docs.python.org/library/re.html -.. _StackOverflow: https://stackoverflow.com/q/36440681/939364 Can I use Basic HTTP Authentication in my spiders? -------------------------------------------------- @@ -206,12 +204,10 @@ I get "Filtered offsite request" messages. How can I fix them? Those messages (logged with ``DEBUG`` level) don't necessarily mean there is a problem, so you may not need to fix them. -Those messages are thrown by the Offsite Spider Middleware, which is a spider -middleware (enabled by default) whose purpose is to filter out requests to -domains outside the ones covered by the spider. - -For more info see: -:class:`~scrapy.spidermiddlewares.offsite.OffsiteMiddleware`. +Those messages are thrown by +:class:`~scrapy.downloadermiddlewares.offsite.OffsiteMiddleware`, which is a +downloader middleware (enabled by default) whose purpose is to filter out +requests to domains outside the ones covered by the spider. What is the recommended way to deploy a Scrapy crawler in production? --------------------------------------------------------------------- diff --git a/docs/topics/benchmarking.rst b/docs/topics/benchmarking.rst index 0643df6a6..b704e54ed 100644 --- a/docs/topics/benchmarking.rst +++ b/docs/topics/benchmarking.rst @@ -24,7 +24,8 @@ You should see an output like this:: 'scrapy.extensions.telnet.TelnetConsole', 'scrapy.extensions.corestats.CoreStats'] 2016-12-16 21:18:49 [scrapy.middleware] INFO: Enabled downloader middlewares: - ['scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware', + ['scrapy.downloadermiddlewares.offsite.OffsiteMiddleware', + 'scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware', 'scrapy.downloadermiddlewares.httpauth.HttpAuthMiddleware', 'scrapy.downloadermiddlewares.downloadtimeout.DownloadTimeoutMiddleware', 'scrapy.downloadermiddlewares.defaultheaders.DefaultHeadersMiddleware', @@ -37,7 +38,6 @@ You should see an output like this:: 'scrapy.downloadermiddlewares.stats.DownloaderStats'] 2016-12-16 21:18:49 [scrapy.middleware] INFO: Enabled spider middlewares: ['scrapy.spidermiddlewares.httperror.HttpErrorMiddleware', - 'scrapy.spidermiddlewares.offsite.OffsiteMiddleware', 'scrapy.spidermiddlewares.referer.RefererMiddleware', 'scrapy.spidermiddlewares.urllength.UrlLengthMiddleware', 'scrapy.spidermiddlewares.depth.DepthMiddleware'] diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index 01eea66c9..11ddf7bec 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -763,6 +763,44 @@ HttpProxyMiddleware Keep in mind this value will take precedence over ``http_proxy``/``https_proxy`` environment variables, and it will also ignore ``no_proxy`` environment variable. +OffsiteMiddleware +----------------- + +.. module:: scrapy.downloadermiddlewares.offsite + :synopsis: Offsite Middleware + +.. class:: OffsiteMiddleware + + .. versionadded:: VERSION + + Filters out Requests for URLs outside the domains covered by the spider. + + This middleware filters out every request whose host names aren't in the + spider's :attr:`~scrapy.Spider.allowed_domains` attribute. + All subdomains of any domain in the list are also allowed. + E.g. the rule ``www.example.org`` will also allow ``bob.www.example.org`` + but not ``www2.example.com`` nor ``example.com``. + + When your spider returns a request for a domain not belonging to those + covered by the spider, this middleware will log a debug message similar to + this one:: + + DEBUG: Filtered offsite request to 'offsite.example': + + To avoid filling the log with too much noise, it will only print one of + these messages for each new domain filtered. So, for example, if another + request for ``offsite.example`` is filtered, no log message will be + printed. But if a request for ``other.example`` is filtered, a message + will be printed (but only for the first request filtered). + + If the spider doesn't define an + :attr:`~scrapy.Spider.allowed_domains` attribute, or the + attribute is empty, the offsite middleware will allow all requests. + + If the request has the :attr:`~scrapy.Request.dont_filter` attribute + set, the offsite middleware will allow the request even if its domain is not + listed in allowed domains. + RedirectMiddleware ------------------ diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index eb24b834a..c5e84c9da 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -674,6 +674,7 @@ Default: .. code-block:: python { + "scrapy.downloadermiddlewares.offsite.OffsiteMiddleware": 50, "scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware": 100, "scrapy.downloadermiddlewares.httpauth.HttpAuthMiddleware": 300, "scrapy.downloadermiddlewares.downloadtimeout.DownloadTimeoutMiddleware": 350, @@ -1605,7 +1606,6 @@ Default: { "scrapy.spidermiddlewares.httperror.HttpErrorMiddleware": 50, - "scrapy.spidermiddlewares.offsite.OffsiteMiddleware": 500, "scrapy.spidermiddlewares.referer.RefererMiddleware": 700, "scrapy.spidermiddlewares.urllength.UrlLengthMiddleware": 800, "scrapy.spidermiddlewares.depth.DepthMiddleware": 900, diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index 9bfd1761c..dc7e11a07 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -343,11 +343,18 @@ request_scheduled .. signal:: request_scheduled .. function:: request_scheduled(request, spider) - Sent when the engine schedules a :class:`~scrapy.Request`, to be - downloaded later. + Sent when the engine is asked to schedule a :class:`~scrapy.Request`, to be + downloaded later, before the request reaches the :ref:`scheduler + `. + + Raise :exc:`~scrapy.exceptions.IgnoreRequest` to drop a request before it + reaches the scheduler. This signal does not support returning deferreds from its handlers. + .. versionadded:: VERSION + Allow dropping requests with :exc:`~scrapy.exceptions.IgnoreRequest`. + :param request: the request that reached the scheduler :type request: :class:`~scrapy.Request` object diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index 3f16efea5..8ddf17a14 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -51,8 +51,8 @@ value. For example, if you want to disable the off-site middleware: .. code-block:: python SPIDER_MIDDLEWARES = { - "myproject.middlewares.CustomSpiderMiddleware": 543, - "scrapy.spidermiddlewares.offsite.OffsiteMiddleware": None, + "scrapy.spidermiddlewares.referer.RefererMiddleware": None, + "myproject.middlewares.CustomRefererSpiderMiddleware": 700, } Finally, keep in mind that some middlewares may need to be enabled through a @@ -313,42 +313,6 @@ Default: ``False`` Pass all responses, regardless of its status code. -OffsiteMiddleware ------------------ - -.. module:: scrapy.spidermiddlewares.offsite - :synopsis: Offsite Spider Middleware - -.. class:: OffsiteMiddleware - - Filters out Requests for URLs outside the domains covered by the spider. - - This middleware filters out every request whose host names aren't in the - spider's :attr:`~scrapy.Spider.allowed_domains` attribute. - All subdomains of any domain in the list are also allowed. - E.g. the rule ``www.example.org`` will also allow ``bob.www.example.org`` - but not ``www2.example.com`` nor ``example.com``. - - When your spider returns a request for a domain not belonging to those - covered by the spider, this middleware will log a debug message similar to - this one:: - - DEBUG: Filtered offsite request to 'www.othersite.com': - - To avoid filling the log with too much noise, it will only print one of - these messages for each new domain filtered. So, for example, if another - request for ``www.othersite.com`` is filtered, no log message will be - printed. But if a request for ``someothersite.com`` is filtered, a message - will be printed (but only for the first request filtered). - - If the spider doesn't define an - :attr:`~scrapy.Spider.allowed_domains` attribute, or the - attribute is empty, the offsite middleware will allow all requests. - - If the request has the :attr:`~scrapy.Request.dont_filter` attribute - set, the offsite middleware will allow the request even if its domain is not - listed in allowed domains. - RefererMiddleware ----------------- diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index 30677fe74..8a0102a51 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -75,7 +75,8 @@ scrapy.Spider An optional list of strings containing domains that this spider is allowed to crawl. Requests for URLs not belonging to the domain names specified in this list (or their subdomains) won't be followed if - :class:`~scrapy.spidermiddlewares.offsite.OffsiteMiddleware` is enabled. + :class:`~scrapy.downloadermiddlewares.offsite.OffsiteMiddleware` is + enabled. Let's say your target url is ``https://www.example.com/1.html``, then add ``'example.com'`` to the list. diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index dd1f56f8c..92967ffc8 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -27,7 +27,7 @@ from twisted.python.failure import Failure from scrapy import signals from scrapy.core.downloader import Downloader from scrapy.core.scraper import Scraper -from scrapy.exceptions import CloseSpider, DontCloseSpider +from scrapy.exceptions import CloseSpider, DontCloseSpider, IgnoreRequest from scrapy.http import Request, Response from scrapy.logformatter import LogFormatter from scrapy.settings import BaseSettings, Settings @@ -35,6 +35,7 @@ from scrapy.signalmanager import SignalManager from scrapy.spiders import Spider from scrapy.utils.log import failure_to_exc_info, logformatter_adapter from scrapy.utils.misc import create_instance, load_object +from scrapy.utils.python import global_object_name from scrapy.utils.reactor import CallLaterOnce if TYPE_CHECKING: @@ -291,9 +292,19 @@ class ExecutionEngine: self.slot.nextcall.schedule() # type: ignore[union-attr] def _schedule_request(self, request: Request, spider: Spider) -> None: - self.signals.send_catch_log( - signals.request_scheduled, request=request, spider=spider + request_scheduled_result = self.signals.send_catch_log( + signals.request_scheduled, + request=request, + spider=spider, + dont_log=IgnoreRequest, ) + for handler, result in request_scheduled_result: + if isinstance(result, Failure) and isinstance(result.value, IgnoreRequest): + logger.debug( + f"Signal handler {global_object_name(handler)} dropped " + f"request {request} before it reached the scheduler." + ) + return if not self.slot.scheduler.enqueue_request(request): # type: ignore[union-attr] self.signals.send_catch_log( signals.request_dropped, request=request, spider=spider diff --git a/scrapy/downloadermiddlewares/offsite.py b/scrapy/downloadermiddlewares/offsite.py new file mode 100644 index 000000000..1e5026925 --- /dev/null +++ b/scrapy/downloadermiddlewares/offsite.py @@ -0,0 +1,77 @@ +import logging +import re +import warnings + +from scrapy import signals +from scrapy.exceptions import IgnoreRequest +from scrapy.utils.httpobj import urlparse_cached + +logger = logging.getLogger(__name__) + + +class OffsiteMiddleware: + @classmethod + def from_crawler(cls, crawler): + o = cls(crawler.stats) + crawler.signals.connect(o.spider_opened, signal=signals.spider_opened) + crawler.signals.connect(o.request_scheduled, signal=signals.request_scheduled) + return o + + def __init__(self, stats): + self.stats = stats + self.domains_seen = set() + + def spider_opened(self, spider): + self.host_regex = self.get_host_regex(spider) + + def request_scheduled(self, request, spider): + self.process_request(request, spider) + + def process_request(self, request, spider): + if request.dont_filter or self.should_follow(request, spider): + return None + domain = urlparse_cached(request).hostname + if domain and domain not in self.domains_seen: + self.domains_seen.add(domain) + logger.debug( + "Filtered offsite request to %(domain)r: %(request)s", + {"domain": domain, "request": request}, + extra={"spider": spider}, + ) + self.stats.inc_value("offsite/domains", spider=spider) + self.stats.inc_value("offsite/filtered", spider=spider) + raise IgnoreRequest + + def should_follow(self, request, spider): + regex = self.host_regex + # hostname can be None for wrong urls (like javascript links) + host = urlparse_cached(request).hostname or "" + return bool(regex.search(host)) + + def get_host_regex(self, spider): + """Override this method to implement a different offsite policy""" + allowed_domains = getattr(spider, "allowed_domains", None) + if not allowed_domains: + return re.compile("") # allow all by default + url_pattern = re.compile(r"^https?://.*$") + port_pattern = re.compile(r":\d+$") + domains = [] + for domain in allowed_domains: + if domain is None: + continue + if url_pattern.match(domain): + message = ( + "allowed_domains accepts only domains, not URLs. " + f"Ignoring URL entry {domain} in allowed_domains." + ) + warnings.warn(message) + elif port_pattern.search(domain): + message = ( + "allowed_domains accepts only domains without ports. " + f"Ignoring entry {domain} in allowed_domains." + ) + warnings.warn(message) + else: + domains.append(re.escape(domain)) + regex = rf'^(.*\.)?({"|".join(domains)})$' + return re.compile(regex) diff --git a/scrapy/extensions/memusage.py b/scrapy/extensions/memusage.py index 221967bda..ca766c938 100644 --- a/scrapy/extensions/memusage.py +++ b/scrapy/extensions/memusage.py @@ -128,9 +128,9 @@ class MemoryUsage: def _send_report(self, rcpts, subject): """send notification mail with some additional useful info""" stats = self.crawler.stats - s = f"Memory usage at engine startup : {stats.get_value('memusage/startup')/1024/1024}M\r\n" - s += f"Maximum memory usage : {stats.get_value('memusage/max')/1024/1024}M\r\n" - s += f"Current memory usage : {self.get_virtual_size()/1024/1024}M\r\n" + s = f"Memory usage at engine startup : {stats.get_value('memusage/startup') / 1024 / 1024}M\r\n" + s += f"Maximum memory usage : {stats.get_value('memusage/max') / 1024 / 1024}M\r\n" + s += f"Current memory usage : {self.get_virtual_size() / 1024 / 1024}M\r\n" s += ( "ENGINE STATUS ------------------------------------------------------- \r\n" diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 7e77884f6..e43ce9d68 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -101,6 +101,7 @@ DOWNLOADER_MIDDLEWARES = {} DOWNLOADER_MIDDLEWARES_BASE = { # Engine side + "scrapy.downloadermiddlewares.offsite.OffsiteMiddleware": 50, "scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware": 100, "scrapy.downloadermiddlewares.httpauth.HttpAuthMiddleware": 300, "scrapy.downloadermiddlewares.downloadtimeout.DownloadTimeoutMiddleware": 350, @@ -299,7 +300,6 @@ SPIDER_MIDDLEWARES = {} SPIDER_MIDDLEWARES_BASE = { # Engine side "scrapy.spidermiddlewares.httperror.HttpErrorMiddleware": 50, - "scrapy.spidermiddlewares.offsite.OffsiteMiddleware": 500, "scrapy.spidermiddlewares.referer.RefererMiddleware": 700, "scrapy.spidermiddlewares.urllength.UrlLengthMiddleware": 800, "scrapy.spidermiddlewares.depth.DepthMiddleware": 900, diff --git a/scrapy/spidermiddlewares/offsite.py b/scrapy/spidermiddlewares/offsite.py index 1a48926b3..243055d89 100644 --- a/scrapy/spidermiddlewares/offsite.py +++ b/scrapy/spidermiddlewares/offsite.py @@ -8,9 +8,16 @@ import re import warnings from scrapy import signals +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Request from scrapy.utils.httpobj import urlparse_cached +warnings.warn( + "The scrapy.spidermiddlewares.offsite module is deprecated, use " + "scrapy.downloadermiddlewares.offsite instead.", + ScrapyDeprecationWarning, +) + logger = logging.getLogger(__name__) diff --git a/scrapy/utils/_compression.py b/scrapy/utils/_compression.py index 14531df3f..7c40d0a02 100644 --- a/scrapy/utils/_compression.py +++ b/scrapy/utils/_compression.py @@ -12,7 +12,6 @@ else: try: brotli.Decompressor.process except AttributeError: - warn( ( "You have brotlipy installed, and Scrapy will use it, but " diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py index 0b5dc324f..20305a75e 100644 --- a/scrapy/utils/python.py +++ b/scrapy/utils/python.py @@ -316,7 +316,7 @@ def global_object_name(obj: Any) -> str: >>> global_object_name(Request) 'scrapy.http.request.Request' """ - return f"{obj.__module__}.{obj.__name__}" + return f"{obj.__module__}.{obj.__qualname__}" if hasattr(sys, "pypy_version_info"): diff --git a/tests/test_downloadermiddleware.py b/tests/test_downloadermiddleware.py index 062e8a8b4..0155c62eb 100644 --- a/tests/test_downloadermiddleware.py +++ b/tests/test_downloadermiddleware.py @@ -22,13 +22,11 @@ class ManagerTestCase(TestCase): self.crawler = get_crawler(Spider, self.settings_dict) self.spider = self.crawler._create_spider("foo") self.mwman = DownloaderMiddlewareManager.from_crawler(self.crawler) - # some mw depends on stats collector - self.crawler.stats.open_spider(self.spider) - return self.mwman.open_spider(self.spider) + self.crawler.engine = self.crawler._create_engine() + return self.crawler.engine.open_spider(self.spider, start_requests=()) def tearDown(self): - self.crawler.stats.close_spider(self.spider, "") - return self.mwman.close_spider(self.spider) + return self.crawler.engine.close_spider(self.spider) def _download(self, request, response=None): """Executes downloader mw manager's download method and returns diff --git a/tests/test_downloadermiddleware_offsite.py b/tests/test_downloadermiddleware_offsite.py new file mode 100644 index 000000000..d4669f450 --- /dev/null +++ b/tests/test_downloadermiddleware_offsite.py @@ -0,0 +1,184 @@ +import pytest + +from scrapy import Request, Spider +from scrapy.downloadermiddlewares.offsite import OffsiteMiddleware +from scrapy.exceptions import IgnoreRequest +from scrapy.utils.test import get_crawler + +UNSET = object() + + +@pytest.mark.parametrize( + ("allowed_domain", "url", "allowed"), + ( + ("example.com", "http://example.com/1", True), + ("example.com", "http://example.org/1", False), + ("example.com", "http://sub.example.com/1", True), + ("sub.example.com", "http://sub.example.com/1", True), + ("sub.example.com", "http://example.com/1", False), + ("example.com", "http://example.com:8000/1", True), + ("example.com", "http://example.org/example.com", False), + ("example.com", "http://example.org/foo.example.com", False), + ("example.com", "http://example.com.example", False), + ("a.example", "http://nota.example", False), + ("b.a.example", "http://notb.a.example", False), + ), +) +def test_process_request_domain_filtering(allowed_domain, url, allowed): + crawler = get_crawler(Spider) + spider = crawler._create_spider(name="a", allowed_domains=[allowed_domain]) + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + request = Request(url) + if allowed: + assert mw.process_request(request, spider) is None + else: + with pytest.raises(IgnoreRequest): + mw.process_request(request, spider) + + +@pytest.mark.parametrize( + ("value", "filtered"), + ( + (UNSET, True), + (None, True), + (False, True), + (True, False), + ), +) +def test_process_request_dont_filter(value, filtered): + crawler = get_crawler(Spider) + spider = crawler._create_spider(name="a", allowed_domains=["a.example"]) + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + kwargs = {} + if value is not UNSET: + kwargs["dont_filter"] = value + request = Request("https://b.example", **kwargs) + if filtered: + with pytest.raises(IgnoreRequest): + mw.process_request(request, spider) + else: + assert mw.process_request(request, spider) is None + + +@pytest.mark.parametrize( + "value", + ( + UNSET, + None, + [], + ), +) +def test_process_request_no_allowed_domains(value): + crawler = get_crawler(Spider) + kwargs = {} + if value is not UNSET: + kwargs["allowed_domains"] = value + spider = crawler._create_spider(name="a", **kwargs) + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + request = Request("https://example.com") + assert mw.process_request(request, spider) is None + + +def test_process_request_invalid_domains(): + crawler = get_crawler(Spider) + allowed_domains = ["a.example", None, "http:////b.example", "//c.example"] + spider = crawler._create_spider(name="a", allowed_domains=allowed_domains) + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + request = Request("https://a.example") + assert mw.process_request(request, spider) is None + for letter in ("b", "c"): + request = Request(f"https://{letter}.example") + with pytest.raises(IgnoreRequest): + mw.process_request(request, spider) + + +@pytest.mark.parametrize( + ("allowed_domain", "url", "allowed"), + ( + ("example.com", "http://example.com/1", True), + ("example.com", "http://example.org/1", False), + ("example.com", "http://sub.example.com/1", True), + ("sub.example.com", "http://sub.example.com/1", True), + ("sub.example.com", "http://example.com/1", False), + ("example.com", "http://example.com:8000/1", True), + ("example.com", "http://example.org/example.com", False), + ("example.com", "http://example.org/foo.example.com", False), + ("example.com", "http://example.com.example", False), + ("a.example", "http://nota.example", False), + ("b.a.example", "http://notb.a.example", False), + ), +) +def test_request_scheduled_domain_filtering(allowed_domain, url, allowed): + crawler = get_crawler(Spider) + spider = crawler._create_spider(name="a", allowed_domains=[allowed_domain]) + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + request = Request(url) + if allowed: + assert mw.request_scheduled(request, spider) is None + else: + with pytest.raises(IgnoreRequest): + mw.request_scheduled(request, spider) + + +@pytest.mark.parametrize( + ("value", "filtered"), + ( + (UNSET, True), + (None, True), + (False, True), + (True, False), + ), +) +def test_request_scheduled_dont_filter(value, filtered): + crawler = get_crawler(Spider) + spider = crawler._create_spider(name="a", allowed_domains=["a.example"]) + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + kwargs = {} + if value is not UNSET: + kwargs["dont_filter"] = value + request = Request("https://b.example", **kwargs) + if filtered: + with pytest.raises(IgnoreRequest): + mw.request_scheduled(request, spider) + else: + assert mw.request_scheduled(request, spider) is None + + +@pytest.mark.parametrize( + "value", + ( + UNSET, + None, + [], + ), +) +def test_request_scheduled_no_allowed_domains(value): + crawler = get_crawler(Spider) + kwargs = {} + if value is not UNSET: + kwargs["allowed_domains"] = value + spider = crawler._create_spider(name="a", **kwargs) + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + request = Request("https://example.com") + assert mw.request_scheduled(request, spider) is None + + +def test_request_scheduled_invalid_domains(): + crawler = get_crawler(Spider) + allowed_domains = ["a.example", None, "http:////b.example", "//c.example"] + spider = crawler._create_spider(name="a", allowed_domains=allowed_domains) + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + request = Request("https://a.example") + assert mw.request_scheduled(request, spider) is None + for letter in ("b", "c"): + request = Request(f"https://{letter}.example") + with pytest.raises(IgnoreRequest): + mw.request_scheduled(request, spider) diff --git a/tests/test_engine.py b/tests/test_engine.py index 8d7afb6a1..33544e8db 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -15,8 +15,10 @@ import subprocess import sys from collections import defaultdict from dataclasses import dataclass +from logging import DEBUG from pathlib import Path from threading import Timer +from unittest.mock import Mock from urllib.parse import urlparse import attr @@ -27,11 +29,13 @@ from twisted.trial import unittest from twisted.web import server, static, util from scrapy import signals -from scrapy.core.engine import ExecutionEngine -from scrapy.exceptions import CloseSpider +from scrapy.core.engine import ExecutionEngine, Slot +from scrapy.core.scheduler import BaseScheduler +from scrapy.exceptions import CloseSpider, IgnoreRequest from scrapy.http import Request from scrapy.item import Field, Item from scrapy.linkextractors import LinkExtractor +from scrapy.signals import request_scheduled from scrapy.spiders import Spider from scrapy.utils.signal import disconnect_all from scrapy.utils.test import get_crawler @@ -467,6 +471,38 @@ class EngineTest(unittest.TestCase): self.assertNotIn(b"Traceback", stderr) +def test_request_scheduled_signal(caplog): + class TestScheduler(BaseScheduler): + def __init__(self): + self.enqueued = [] + + def enqueue_request(self, request: Request) -> bool: + self.enqueued.append(request) + return True + + def signal_handler(request: Request, spider: Spider) -> None: + if "drop" in request.url: + raise IgnoreRequest + + spider = TestSpider() + crawler = get_crawler(spider.__class__) + engine = ExecutionEngine(crawler, lambda _: None) + engine.downloader._slot_gc_loop.stop() + scheduler = TestScheduler() + engine.slot = Slot((), None, Mock(), scheduler) + crawler.signals.connect(signal_handler, request_scheduled) + keep_request = Request("https://keep.example") + engine._schedule_request(keep_request, spider) + drop_request = Request("https://drop.example") + caplog.set_level(DEBUG) + engine._schedule_request(drop_request, spider) + assert scheduler.enqueued == [ + keep_request + ], f"{scheduler.enqueued!r} != [{keep_request!r}]" + assert "dropped request " in caplog.text + crawler.signals.disconnect(signal_handler, request_scheduled) + + if __name__ == "__main__": if len(sys.argv) > 1 and sys.argv[1] == "runserver": start_test_site(debug=True) diff --git a/tests/test_utils_iterators.py b/tests/test_utils_iterators.py index 505cc276c..ee22e6675 100644 --- a/tests/test_utils_iterators.py +++ b/tests/test_utils_iterators.py @@ -546,6 +546,6 @@ class TestHelper(unittest.TestCase): def _assert_type_and_value(self, a, b, obj): self.assertTrue( - type(a) is type(b), f"Got {type(a)}, expected {type(b)} for { obj!r}" + type(a) is type(b), f"Got {type(a)}, expected {type(b)} for {obj!r}" ) self.assertEqual(a, b)