From 0a152764fb5aac7fc5cf575c63155a3374caca7b Mon Sep 17 00:00:00 2001 From: Diogo Castro Date: Sun, 21 Jun 2026 23:01:17 -0300 Subject: [PATCH] fix: Addressing feedbacks, changing error message, and adding tests --- scrapy/downloadermiddlewares/offsite.py | 14 ++--- tests/test_downloadermiddleware_offsite.py | 73 +++++++++++++++++++--- 2 files changed, 72 insertions(+), 15 deletions(-) diff --git a/scrapy/downloadermiddlewares/offsite.py b/scrapy/downloadermiddlewares/offsite.py index ea2c2f030..fc9f35a66 100644 --- a/scrapy/downloadermiddlewares/offsite.py +++ b/scrapy/downloadermiddlewares/offsite.py @@ -49,12 +49,12 @@ class OffsiteMiddleware: {"error": exc}, extra={"spider": spider}, ) - if self.crawler.engine: - _schedule_coro( - self.crawler.engine.close_spider_async( - reason="invalid_domain_configuration" - ) + assert self.crawler.engine + _schedule_coro( + self.crawler.engine.close_spider_async( + reason="invalid_domain_configuration" ) + ) def request_scheduled(self, request: Request, spider: Spider) -> None: self.process_request(request) @@ -110,12 +110,12 @@ class OffsiteMiddleware: if url_pattern.match(domain): raise ValueError( f"{domains_type} accepts only domains, not URLs. " - f"Ignoring URL entry {domain} in {domains_type}." + f"Got URL entry {domain} in {domains_type}." ) if port_pattern.search(domain): raise ValueError( f"{domains_type} accepts only domains without ports. " - f"Ignoring entry {domain} in {domains_type}." + f"Got entry {domain} in {domains_type}." ) valid_domains.append(re.escape(domain)) return valid_domains diff --git a/tests/test_downloadermiddleware_offsite.py b/tests/test_downloadermiddleware_offsite.py index 0d7e8982b..20172c02c 100644 --- a/tests/test_downloadermiddleware_offsite.py +++ b/tests/test_downloadermiddleware_offsite.py @@ -116,12 +116,23 @@ def test_process_request_no_allowed_domains(value): assert mw.process_request(request) is None -def test_process_request_invalid_domains(caplog): +@pytest.mark.parametrize( + "allowed_domains", + [ + ["a.example", None], + ["a.example", "http://b.example"], + ["a.example", "c.example:8080"], + ], +) +def test_process_request_invalid_domains(allowed_domains, caplog): crawler = get_crawler(Spider) - allowed_domains = ["a.example", None, "http:////b.example", "//c.example"] crawler.spider = crawler._create_spider(name="a", allowed_domains=allowed_domains) mw = OffsiteMiddleware.from_crawler(crawler) - with caplog.at_level(logging.ERROR): + crawler.engine = AsyncMock() + with ( + patch("scrapy.downloadermiddlewares.offsite._schedule_coro"), + caplog.at_level(logging.ERROR), + ): mw.spider_opened(crawler.spider) assert "Invalid domain configuration" in caplog.text @@ -217,12 +228,23 @@ def test_request_scheduled_no_allowed_domains(value): assert mw.request_scheduled(request, crawler.spider) is None -def test_request_scheduled_invalid_domains(caplog): +@pytest.mark.parametrize( + "allowed_domains", + [ + ["a.example", None], + ["a.example", "http://b.example"], + ["a.example", "c.example:8080"], + ], +) +def test_request_scheduled_invalid_domains(allowed_domains, caplog): crawler = get_crawler(Spider) - allowed_domains = ["a.example", None, "http:////b.example", "//c.example"] crawler.spider = crawler._create_spider(name="a", allowed_domains=allowed_domains) mw = OffsiteMiddleware.from_crawler(crawler) - with caplog.at_level(logging.ERROR): + crawler.engine = AsyncMock() + with ( + patch("scrapy.downloadermiddlewares.offsite._schedule_coro"), + caplog.at_level(logging.ERROR), + ): mw.spider_opened(crawler.spider) assert "Invalid domain configuration" in caplog.text @@ -325,7 +347,11 @@ def test_process_request_invalid_disallowed_domains(disallowed_domains, caplog): ) mw = OffsiteMiddleware.from_crawler(crawler) - with caplog.at_level(logging.ERROR): + crawler.engine = AsyncMock() + with ( + patch("scrapy.downloadermiddlewares.offsite._schedule_coro"), + caplog.at_level(logging.ERROR), + ): mw.spider_opened(crawler.spider) assert "Invalid domain configuration" in caplog.text @@ -430,6 +456,37 @@ def test_request_scheduled_invalid_disallowed_domains(disallowed_domains, caplog ) mw = OffsiteMiddleware.from_crawler(crawler) - with caplog.at_level(logging.ERROR): + crawler.engine = AsyncMock() + with ( + patch("scrapy.downloadermiddlewares.offsite._schedule_coro"), + caplog.at_level(logging.ERROR), + ): mw.spider_opened(crawler.spider) assert "Invalid domain configuration" in caplog.text + + +@pytest.mark.parametrize( + ("url", "filtered"), + [ + ("http://example.com/page", False), + ("http://sub.example.com/page", False), + ("http://ads.example.com/page", True), + ("http://sub.ads.example.com/page", True), + ("http://other.com/page", True), + ], +) +def test_process_request_allowed_and_disallowed_domains(url, filtered): + crawler = get_crawler(Spider) + crawler.spider = crawler._create_spider( + name="a", + allowed_domains=["example.com"], + disallowed_domains=["ads.example.com"], + ) + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(crawler.spider) + request = Request(url) + if filtered: + with pytest.raises(IgnoreRequest): + mw.process_request(request) + else: + assert mw.process_request(request) is None