From baa579df625d698bebd52104bcd52e2682a80f0e Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 5 Aug 2025 12:21:08 +0500 Subject: [PATCH] Deprecate various `spider` args in `MiddlewareManager` subclasses (#6984) * Add a crawler argument to MiddlewareManager.__init__(). * Deprecate the spider argument in some MiddlewareManager subclasses. * Remove the spider argument from ExecutionEngine.open_spider_async(). * Remove the spider argument from Scraper.open_spider(). * Fix the Scraper.close_spider() type hint. * Remove the spider argument from Downloader internals. * Deprecate the spider arg of download_func. * Deprecate the spider arg of Downloader.fetch(). * Deprecate the spider arg of ItemPipelineManager.process_item(). * Fix pylint. * Tests for MiddlewareManager spider code. * Deprecated spider arg handling for custom pipeline managers. * Add a test for the deprecated spider arg in a custom downloader. * Remove TODO. * Fix pylint. --- scrapy/commands/parse.py | 7 +- scrapy/core/downloader/__init__.py | 54 +++++--- scrapy/core/downloader/handlers/__init__.py | 16 ++- scrapy/core/downloader/middleware.py | 32 ++++- scrapy/core/engine.py | 54 +++++--- scrapy/core/scraper.py | 65 +++++++-- scrapy/core/spidermw.py | 71 +++++----- scrapy/crawler.py | 4 +- scrapy/middleware.py | 68 +++++++++- scrapy/pipelines/__init__.py | 7 +- scrapy/shell.py | 2 +- scrapy/utils/deprecate.py | 28 +++- scrapy/utils/python.py | 31 +++-- tests/test_core_downloader.py | 18 ++- tests/test_downloadermiddleware.py | 115 ++++++++++------ tests/test_downloaderslotssettings.py | 6 +- tests/test_engine.py | 15 ++- tests/test_middleware.py | 142 +++++++++++++++++--- tests/test_pipelines.py | 74 +++++++++- tests/test_spidermiddleware.py | 26 ++-- tests/test_utils_python.py | 2 +- 21 files changed, 628 insertions(+), 209 deletions(-) diff --git a/scrapy/commands/parse.py b/scrapy/commands/parse.py index c4b3d2af9..9f6c752d7 100644 --- a/scrapy/commands/parse.py +++ b/scrapy/commands/parse.py @@ -16,6 +16,7 @@ from scrapy.http import Request, Response from scrapy.utils import display from scrapy.utils.asyncgen import collect_asyncgen from scrapy.utils.defer import aiter_errback, deferred_from_coro +from scrapy.utils.deprecate import argument_is_required from scrapy.utils.log import failure_to_exc_info from scrapy.utils.misc import arg_to_iter from scrapy.utils.spider import spidercls_for_request @@ -284,8 +285,12 @@ class Command(BaseRunSpiderCommand): if opts.pipelines: assert self.pcrawler.engine itemproc = self.pcrawler.engine.scraper.itemproc + needs_spider = argument_is_required(itemproc.process_item, "spider") for item in items: - itemproc.process_item(item, spider) + if needs_spider: + itemproc.process_item(item, spider) + else: + itemproc.process_item(item) self.add_items(depth, items) self.add_requests(depth, requests) diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index 4c9fed3bc..627e16331 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -105,6 +105,7 @@ class Downloader: DOWNLOAD_SLOT = "download_slot" def __init__(self, crawler: Crawler): + self.crawler: Crawler = crawler self.settings: BaseSettings = crawler.settings self.signals: SignalManager = crawler.signals self.slots: dict[str, Slot] = {} @@ -129,27 +130,34 @@ class Downloader: @inlineCallbacks def fetch( - self, request: Request, spider: Spider + self, request: Request, spider: Spider | None = None ) -> Generator[Deferred[Any], Any, Response | Request]: + if spider is not None: + warnings.warn( + "Passing a 'spider' argument to Downloader.fetch() is deprecated.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) self.active.add(request) try: - return ( - yield self.middleware.download(self._enqueue_request, request, spider) - ) + return (yield self.middleware.download(self._enqueue_request, request)) finally: self.active.remove(request) def needs_backout(self) -> bool: return len(self.active) >= self.total_concurrency - def _get_slot(self, request: Request, spider: Spider) -> tuple[str, Slot]: + def _get_slot(self, request: Request) -> tuple[str, Slot]: key = self.get_slot_key(request) if key not in self.slots: + assert self.crawler.spider slot_settings = self.per_slot_settings.get(key, {}) conc = ( self.ip_concurrency if self.ip_concurrency else self.domain_concurrency ) - conc, delay = _get_concurrency_delay(conc, spider, self.settings) + conc, delay = _get_concurrency_delay( + conc, self.crawler.spider, self.settings + ) conc, delay = ( slot_settings.get("concurrency", conc), slot_settings.get("delay", delay), @@ -180,23 +188,25 @@ class Downloader: @inlineCallbacks def _enqueue_request( - self, request: Request, spider: Spider + self, request: Request ) -> Generator[Deferred[Any], Any, Response]: - key, slot = self._get_slot(request, spider) + key, slot = self._get_slot(request) request.meta[self.DOWNLOAD_SLOT] = key slot.active.add(request) self.signals.send_catch_log( - signal=signals.request_reached_downloader, request=request, spider=spider + signal=signals.request_reached_downloader, + request=request, + spider=self.crawler.spider, ) d: Deferred[Response] = Deferred() slot.queue.append((request, d)) - self._process_queue(spider, slot) + self._process_queue(slot) try: return (yield d) finally: slot.active.remove(request) - def _process_queue(self, spider: Spider, slot: Slot) -> None: + def _process_queue(self, slot: Slot) -> None: if slot.latercall: # block processing until slot.latercall is called return @@ -207,31 +217,31 @@ class Downloader: if delay: penalty = delay - now + slot.lastseen if penalty > 0: - slot.latercall = call_later(penalty, self._latercall, spider, slot) + slot.latercall = call_later(penalty, self._latercall, slot) return # Process enqueued requests if there are free slots to transfer for this slot while slot.queue and slot.free_transfer_slots() > 0: slot.lastseen = now request, deferred = slot.queue.popleft() - dfd = deferred_from_coro(self._download(slot, request, spider)) + dfd = deferred_from_coro(self._download(slot, request)) dfd.chainDeferred(deferred) # prevent burst if inter-request delays were configured if delay: - self._process_queue(spider, slot) + self._process_queue(slot) break - def _latercall(self, spider: Spider, slot: Slot) -> None: + def _latercall(self, slot: Slot) -> None: slot.latercall = None - self._process_queue(spider, slot) + self._process_queue(slot) - async def _download(self, slot: Slot, request: Request, spider: Spider) -> Response: + async def _download(self, slot: Slot, request: Request) -> Response: # The order is very important for the following logic. Do not change! slot.transferring.add(request) try: # 1. Download the response response: Response = await maybe_deferred_to_future( - self.handlers.download_request(request, spider) + self.handlers.download_request(request) ) # 2. Notify response_downloaded listeners about the recent download # before querying queue for next request @@ -239,7 +249,7 @@ class Downloader: signal=signals.response_downloaded, response=response, request=request, - spider=spider, + spider=self.crawler.spider, ) return response except Exception: @@ -251,9 +261,11 @@ class Downloader: # following requests (perhaps those which came from the downloader # middleware itself) slot.transferring.remove(request) - self._process_queue(spider, slot) + self._process_queue(slot) self.signals.send_catch_log( - signal=signals.request_left_downloader, request=request, spider=spider + signal=signals.request_left_downloader, + request=request, + spider=self.crawler.spider, ) def close(self) -> None: diff --git a/scrapy/core/downloader/handlers/__init__.py b/scrapy/core/downloader/handlers/__init__.py index 902f200b8..fe3c780e3 100644 --- a/scrapy/core/downloader/handlers/__init__.py +++ b/scrapy/core/downloader/handlers/__init__.py @@ -3,12 +3,13 @@ from __future__ import annotations import logging +import warnings from typing import TYPE_CHECKING, Any, Protocol, cast from twisted.internet import defer from scrapy import Request, Spider, signals -from scrapy.exceptions import NotConfigured, NotSupported +from scrapy.exceptions import NotConfigured, NotSupported, ScrapyDeprecationWarning from scrapy.utils.httpobj import urlparse_cached from scrapy.utils.misc import build_from_crawler, load_object from scrapy.utils.python import without_none_values @@ -93,14 +94,23 @@ class DownloadHandlers: self._handlers[scheme] = dh return dh - def download_request(self, request: Request, spider: Spider) -> Deferred[Response]: + def download_request( + self, request: Request, spider: Spider | None = None + ) -> Deferred[Response]: + if spider is not None: + warnings.warn( + "Passing a 'spider' argument to DownloadHandlers.download_request() is deprecated.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) scheme = urlparse_cached(request).scheme handler = self._get_handler(scheme) if not handler: raise NotSupported( f"Unsupported URL scheme '{scheme}': {self._notconfigured[scheme]}" ) - return handler.download_request(request, spider) + assert self._crawler.spider + return handler.download_request(request, self._crawler.spider) @defer.inlineCallbacks def _close(self, *_a: Any, **_kw: Any) -> Generator[Deferred[Any], Any, None]: diff --git a/scrapy/core/downloader/middleware.py b/scrapy/core/downloader/middleware.py index 2e892cff7..376b0cb31 100644 --- a/scrapy/core/downloader/middleware.py +++ b/scrapy/core/downloader/middleware.py @@ -6,15 +6,17 @@ See documentation in docs/topics/downloader-middleware.rst from __future__ import annotations +import warnings from typing import TYPE_CHECKING, Any, cast from twisted.internet.defer import Deferred, inlineCallbacks -from scrapy.exceptions import _InvalidOutput +from scrapy.exceptions import ScrapyDeprecationWarning, _InvalidOutput from scrapy.http import Request, Response from scrapy.middleware import MiddlewareManager from scrapy.utils.conf import build_component_list from scrapy.utils.defer import _defer_sleep, deferred_from_coro +from scrapy.utils.deprecate import argument_is_required if TYPE_CHECKING: from collections.abc import Callable, Generator @@ -41,10 +43,21 @@ class DownloaderMiddlewareManager(MiddlewareManager): @inlineCallbacks def download( self, - download_func: Callable[[Request, Spider], Deferred[Response]], + download_func: Callable[[Request], Deferred[Response]], request: Request, - spider: Spider, + spider: Spider | None = None, ) -> Generator[Deferred[Any], Any, Response | Request]: + if argument_is_required(download_func, "spider"): + warnings.warn( + "The spider argument of download_func is deprecated" + " and will not be passed in the future Scrapy versions.", + ScrapyDeprecationWarning, + stacklevel=2, + ) + need_spider_arg = True + else: + need_spider_arg = False + @inlineCallbacks def process_request( request: Request, @@ -52,7 +65,7 @@ class DownloaderMiddlewareManager(MiddlewareManager): for method in self.methods["process_request"]: method = cast("Callable", method) response = yield deferred_from_coro( - method(request=request, spider=spider) + method(request=request, spider=self._spider) ) if response is not None and not isinstance( response, (Response, Request) @@ -63,7 +76,9 @@ class DownloaderMiddlewareManager(MiddlewareManager): ) if response: return response - return (yield download_func(request, spider)) + if need_spider_arg: + return (yield download_func(request, self._spider)) # type: ignore[call-arg] + return (yield download_func(request)) @inlineCallbacks def process_response( @@ -77,7 +92,7 @@ class DownloaderMiddlewareManager(MiddlewareManager): for method in self.methods["process_response"]: method = cast("Callable", method) response = yield deferred_from_coro( - method(request=request, response=response, spider=spider) + method(request=request, response=response, spider=self._spider) ) if not isinstance(response, (Response, Request)): raise _InvalidOutput( @@ -95,7 +110,7 @@ class DownloaderMiddlewareManager(MiddlewareManager): for method in self.methods["process_exception"]: method = cast("Callable", method) response = yield deferred_from_coro( - method(request=request, exception=exception, spider=spider) + method(request=request, exception=exception, spider=self._spider) ) if response is not None and not isinstance( response, (Response, Request) @@ -108,6 +123,9 @@ class DownloaderMiddlewareManager(MiddlewareManager): return response raise exception + if spider: + self._warn_spider_arg("download") + self._set_compat_spider(spider) try: result: Response | Request = yield process_request(request) except Exception as ex: diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index e6ce58376..520bb6d6b 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -34,8 +34,10 @@ from scrapy.utils.defer import ( deferred_from_coro, maybe_deferred_to_future, ) +from scrapy.utils.deprecate import argument_is_required from scrapy.utils.log import failure_to_exc_info, logformatter_adapter from scrapy.utils.misc import build_from_crawler, load_object +from scrapy.utils.python import global_object_name from scrapy.utils.reactor import CallLaterOnce if TYPE_CHECKING: @@ -121,6 +123,17 @@ class ExecutionEngine: crawler.settings ) self.downloader: Downloader = downloader_cls(crawler) + self._downloader_fetch_needs_spider: bool = argument_is_required( + self.downloader.fetch, "spider" + ) + if self._downloader_fetch_needs_spider: + warnings.warn( + f"The fetch() method of {global_object_name(downloader_cls)} requires a spider argument," + f" this is deprecated and the argument will not be passed in the future Scrapy versions.", + ScrapyDeprecationWarning, + stacklevel=2, + ) + self.scraper: Scraper = Scraper(crawler) except Exception: self.close() @@ -419,9 +432,11 @@ class ExecutionEngine: self._slot.add_request(request) try: - result: Response | Request = yield self.downloader.fetch( - request, self.spider - ) + result: Response | Request + if self._downloader_fetch_needs_spider: + result = yield self.downloader.fetch(request, self.spider) + else: + result = yield self.downloader.fetch(request) if not isinstance(result, (Response, Request)): raise TypeError( f"Incorrect type: expected Response or Request, got {type(result)}: {result!r}" @@ -429,7 +444,6 @@ class ExecutionEngine: if isinstance(result, Response): if result.request is None: result.request = request - assert self.spider is not None logkws = self.logformatter.crawled(result.request, result, self.spider) if logkws is not None: logger.log( @@ -451,30 +465,28 @@ class ExecutionEngine: ScrapyDeprecationWarning, stacklevel=2, ) - return deferred_from_coro( - self.open_spider_async(spider, close_if_idle=close_if_idle) - ) + return deferred_from_coro(self.open_spider_async(close_if_idle=close_if_idle)) - async def open_spider_async( - self, - spider: Spider, - *, - close_if_idle: bool = True, - ) -> None: + async def open_spider_async(self, *, close_if_idle: bool = True) -> None: + assert self.crawler.spider if self._slot is not None: - raise RuntimeError(f"No free spider slot when opening {spider.name!r}") - logger.info("Spider opened", extra={"spider": spider}) - self.spider = spider + raise RuntimeError( + f"No free spider slot when opening {self.crawler.spider.name!r}" + ) + logger.info("Spider opened", extra={"spider": self.crawler.spider}) + self.spider = self.crawler.spider nextcall = CallLaterOnce(self._start_scheduled_requests) scheduler = build_from_crawler(self.scheduler_cls, self.crawler) self._slot = _Slot(close_if_idle, nextcall, scheduler) - self._start = await self.scraper.spidermw.process_start(spider) - if hasattr(scheduler, "open") and (d := scheduler.open(spider)): + self._start = await self.scraper.spidermw.process_start() + if hasattr(scheduler, "open") and (d := scheduler.open(self.crawler.spider)): await maybe_deferred_to_future(d) - await maybe_deferred_to_future(self.scraper.open_spider(spider)) + await maybe_deferred_to_future(self.scraper.open_spider()) assert self.crawler.stats - self.crawler.stats.open_spider(spider) - await self.signals.send_catch_log_async(signals.spider_opened, spider=spider) + self.crawler.stats.open_spider(self.crawler.spider) + await self.signals.send_catch_log_async( + signals.spider_opened, spider=self.crawler.spider + ) def _spider_idle(self) -> None: """ diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index 47d9ae2e2..90236bec5 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -32,8 +32,10 @@ from scrapy.utils.defer import ( parallel, parallel_async, ) +from scrapy.utils.deprecate import argument_is_required from scrapy.utils.log import failure_to_exc_info, logformatter_adapter from scrapy.utils.misc import load_object, warn_on_generator_with_return_value +from scrapy.utils.python import global_object_name from scrapy.utils.spider import iterate_spider_output if TYPE_CHECKING: @@ -106,6 +108,22 @@ class Scraper: crawler.settings["ITEM_PROCESSOR"] ) self.itemproc: ItemPipelineManager = itemproc_cls.from_crawler(crawler) + self._itemproc_needs_spider: dict[str, bool] = {} + for method in ( + "open_spider", + "close_spider", + "process_item", + ): + self._itemproc_needs_spider[method] = argument_is_required( + getattr(self.itemproc, method), "spider" + ) + if self._itemproc_needs_spider[method]: + warnings.warn( + f"The {method}() method of {global_object_name(itemproc_cls)} requires a spider argument," + f" this is deprecated and the argument will not be passed in the future Scrapy versions.", + ScrapyDeprecationWarning, + stacklevel=2, + ) self.concurrent_items: int = crawler.settings.getint("CONCURRENT_ITEMS") self.crawler: Crawler = crawler self.signals: SignalManager = crawler.signals @@ -113,13 +131,29 @@ class Scraper: self.logformatter: LogFormatter = crawler.logformatter @deferred_f_from_coro_f - async def open_spider(self, spider: Spider) -> None: - """Open the given spider for scraping and allocate resources for it""" - self.slot = Slot(self.crawler.settings.getint("SCRAPER_SLOT_MAX_ACTIVE_SIZE")) - await maybe_deferred_to_future(self.itemproc.open_spider(spider)) + async def open_spider(self, spider: Spider | None = None) -> None: + """Open the spider for scraping and allocate resources for it""" + if spider is not None: + warnings.warn( + "Passing a 'spider' argument to Scraper.open_spider() is deprecated.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) - def close_spider(self, spider: Spider | None = None) -> Deferred[Spider]: - """Close a spider being scraped and release its resources""" + self.slot = Slot(self.crawler.settings.getint("SCRAPER_SLOT_MAX_ACTIVE_SIZE")) + if not self.crawler.spider: + raise RuntimeError( + "Scraper.open_spider() called before Crawler.spider is set." + ) + if self._itemproc_needs_spider["open_spider"]: + await maybe_deferred_to_future( + self.itemproc.open_spider(self.crawler.spider) + ) + else: + await maybe_deferred_to_future(self.itemproc.open_spider()) + + def close_spider(self, spider: Spider | None = None) -> Deferred[list[None]]: + """Close the spider being scraped and release its resources""" if spider is not None: warnings.warn( "Passing a 'spider' argument to Scraper.close_spider() is deprecated.", @@ -130,9 +164,12 @@ class Scraper: if self.slot is None: raise RuntimeError("Scraper slot not assigned") self.slot.closing = Deferred() - self.slot.closing.addCallback(self.itemproc.close_spider) + if self._itemproc_needs_spider["close_spider"]: + d = self.slot.closing.addCallback(self.itemproc.close_spider) + else: + d = self.slot.closing.addCallback(lambda _: self.itemproc.close_spider()) self._check_if_closing() - return self.slot.closing + return d def is_idle(self) -> bool: """Return True if there isn't any more spiders to process""" @@ -187,13 +224,12 @@ class Scraper: f"Incorrect type: expected Response or Failure, got {type(result)}: {result!r}" ) - assert self.crawler.spider output: Iterable[Any] | AsyncIterator[Any] if isinstance(result, Response): try: # call the spider middlewares and the request callback with the response output = await self.spidermw.scrape_response_async( - self.call_spider_async, result, request, self.crawler.spider + self.call_spider_async, result, request ) except Exception: self.handle_spider_error(Failure(), request, result) @@ -206,6 +242,7 @@ class Scraper: output = await self.call_spider_async(result, request) except Exception as spider_exc: # the errback didn't silence the exception + assert self.crawler.spider if not result.check(IgnoreRequest): logkws = self.logformatter.download_error( result, request, self.crawler.spider @@ -412,9 +449,11 @@ class Scraper: assert self.crawler.spider is not None # typing self.slot.itemproc_size += 1 try: - output = await maybe_deferred_to_future( - self.itemproc.process_item(item, self.crawler.spider) - ) + if self._itemproc_needs_spider["process_item"]: + d = self.itemproc.process_item(item, self.crawler.spider) + else: + d = self.itemproc.process_item(item) + output = await maybe_deferred_to_future(d) except DropItem as ex: logkws = self.logformatter.dropped(item, ex, response, self.crawler.spider) if logkws is not None: diff --git a/scrapy/core/spidermw.py b/scrapy/core/spidermw.py index a2513f4ec..4bd45840f 100644 --- a/scrapy/core/spidermw.py +++ b/scrapy/core/spidermw.py @@ -33,6 +33,7 @@ from scrapy.utils.python import MutableAsyncChain, MutableChain, global_object_n if TYPE_CHECKING: from collections.abc import Generator + from scrapy.crawler import Crawler from scrapy.settings import BaseSettings @@ -57,12 +58,12 @@ class SpiderMiddlewareManager(MiddlewareManager): def _get_mwlist_from_settings(cls, settings: BaseSettings) -> list[Any]: return build_component_list(settings.getwithbase("SPIDER_MIDDLEWARES")) - def __init__(self, *middlewares: Any) -> None: + def __init__(self, *middlewares: Any, crawler: Crawler | None = None) -> None: self._check_deprecated_process_start_requests_use(middlewares) - super().__init__(*middlewares) + super().__init__(*middlewares, crawler=crawler) def _check_deprecated_process_start_requests_use( - self, middlewares: tuple[Any] + self, middlewares: tuple[Any, ...] ) -> None: deprecated_middlewares = [ middleware @@ -136,12 +137,11 @@ class SpiderMiddlewareManager(MiddlewareManager): scrape_func: ScrapeFunc[_T], response: Response, request: Request, - spider: Spider, ) -> Iterable[_T] | AsyncIterator[_T]: for method in self.methods["process_spider_input"]: method = cast("Callable", method) try: - result = method(response=response, spider=spider) + result = method(response=response, spider=self._spider) if result is not None: msg = ( f"{global_object_name(method)} must return None " @@ -157,7 +157,6 @@ class SpiderMiddlewareManager(MiddlewareManager): def _evaluate_iterable( self, response: Response, - spider: Spider, iterable: Iterable[_T] | AsyncIterator[_T], exception_processor_index: int, recover_to: MutableChain[_T] | MutableAsyncChain[_T], @@ -169,7 +168,7 @@ class SpiderMiddlewareManager(MiddlewareManager): exception_result = cast( "Union[Failure, MutableChain[_T]]", self._process_spider_exception( - response, spider, ex, exception_processor_index + response, ex, exception_processor_index ), ) if isinstance(exception_result, Failure): @@ -185,7 +184,7 @@ class SpiderMiddlewareManager(MiddlewareManager): exception_result = cast( "Union[Failure, MutableAsyncChain[_T]]", self._process_spider_exception( - response, spider, ex, exception_processor_index + response, ex, exception_processor_index ), ) if isinstance(exception_result, Failure): @@ -200,7 +199,6 @@ class SpiderMiddlewareManager(MiddlewareManager): def _process_spider_exception( self, response: Response, - spider: Spider, exception: Exception, start_index: int = 0, ) -> MutableChain[_T] | MutableAsyncChain[_T]: @@ -214,14 +212,12 @@ class SpiderMiddlewareManager(MiddlewareManager): if method is None: continue method = cast("Callable", method) - result = method(response=response, exception=exception, spider=spider) + result = method(response=response, exception=exception, spider=self._spider) if _isiterable(result): # stop exception handling by handing control over to the # process_spider_output chain if an iterable has been returned dfd: Deferred[MutableChain[_T] | MutableAsyncChain[_T]] = ( - self._process_spider_output( - response, spider, result, method_index + 1 - ) + self._process_spider_output(response, result, method_index + 1) ) # _process_spider_output() returns a Deferred only because of downgrading so this can be # simplified when downgrading is removed. @@ -250,7 +246,6 @@ class SpiderMiddlewareManager(MiddlewareManager): def _process_spider_output( self, response: Response, - spider: Spider, result: Iterable[_T] | AsyncIterator[_T], start_index: int = 0, ) -> Generator[Deferred[Any], Any, MutableChain[_T] | MutableAsyncChain[_T]]: @@ -303,19 +298,17 @@ class SpiderMiddlewareManager(MiddlewareManager): ) recovered = MutableChain(recovered_collected) # might fail directly if the output value is not a generator - result = method(response=response, result=result, spider=spider) + result = method(response=response, result=result, spider=self._spider) except Exception as ex: exception_result: Failure | MutableChain[_T] | MutableAsyncChain[_T] = ( - self._process_spider_exception( - response, spider, ex, method_index + 1 - ) + self._process_spider_exception(response, ex, method_index + 1) ) if isinstance(exception_result, Failure): raise return exception_result if _isiterable(result): result = self._evaluate_iterable( - response, spider, result, method_index + 1, recovered + response, result, method_index + 1, recovered ) else: if iscoroutine(result): @@ -339,7 +332,6 @@ class SpiderMiddlewareManager(MiddlewareManager): async def _process_callback_output( self, response: Response, - spider: Spider, result: Iterable[_T] | AsyncIterator[_T], ) -> MutableChain[_T] | MutableAsyncChain[_T]: recovered: MutableChain[_T] | MutableAsyncChain[_T] @@ -347,11 +339,11 @@ class SpiderMiddlewareManager(MiddlewareManager): recovered = MutableAsyncChain() else: recovered = MutableChain() - result = self._evaluate_iterable(response, spider, result, 0, recovered) + result = self._evaluate_iterable(response, result, 0, recovered) result = await maybe_deferred_to_future( cast( "Deferred[Iterable[_T] | AsyncIterator[_T]]", - self._process_spider_output(response, spider, result), + self._process_spider_output(response, result), ) ) if isinstance(result, AsyncIterator): @@ -383,8 +375,9 @@ class SpiderMiddlewareManager(MiddlewareManager): ) -> Iterable[_T] | AsyncIterator[_T]: return await maybe_deferred_to_future(scrape_func(response, request)) + self._set_compat_spider(spider) return deferred_from_coro( - self.scrape_response_async(scrape_func_wrapped, response, request, spider) + self.scrape_response_async(scrape_func_wrapped, response, request) ) async def scrape_response_async( @@ -392,46 +385,56 @@ class SpiderMiddlewareManager(MiddlewareManager): scrape_func: ScrapeFunc[_T], response: Response, request: Request, - spider: Spider, ) -> MutableChain[_T] | MutableAsyncChain[_T]: + if not self.crawler: + raise RuntimeError( + "scrape_response_async() called on a SpiderMiddlewareManager" + " instance created without a crawler." + ) + async def process_callback_output( result: Iterable[_T] | AsyncIterator[_T], ) -> MutableChain[_T] | MutableAsyncChain[_T]: - return await self._process_callback_output(response, spider, result) + return await self._process_callback_output(response, result) def process_spider_exception( exception: Exception, ) -> MutableChain[_T] | MutableAsyncChain[_T]: - return self._process_spider_exception(response, spider, exception) + return self._process_spider_exception(response, exception) try: it: Iterable[_T] | AsyncIterator[_T] = await self._process_spider_input( - scrape_func, response, request, spider + scrape_func, response, request ) return await process_callback_output(it) except Exception as ex: await _defer_sleep_async() return process_spider_exception(ex) - async def process_start(self, spider: Spider) -> AsyncIterator[Any] | None: - self._check_deprecated_start_requests_use(spider) + async def process_start( + self, spider: Spider | None = None + ) -> AsyncIterator[Any] | None: + if spider: + self._warn_spider_arg("process_start") + self._set_compat_spider(spider) + self._check_deprecated_start_requests_use() if self._use_start_requests: - sync_start = iter(spider.start_requests()) + sync_start = iter(self._spider.start_requests()) sync_start = await maybe_deferred_to_future( - self._process_chain("process_start_requests", sync_start, spider) + self._process_chain("process_start_requests", sync_start, self._spider) ) start: AsyncIterator[Any] = as_async_generator(sync_start) else: - start = spider.start() + start = self._spider.start() start = await maybe_deferred_to_future( self._process_chain("process_start", start) ) return start - def _check_deprecated_start_requests_use(self, spider: Spider): + def _check_deprecated_start_requests_use(self): start_requests_cls = None start_cls = None - spidercls = spider.__class__ + spidercls = self._spider.__class__ mro = spidercls.__mro__ for cls in mro: diff --git a/scrapy/crawler.py b/scrapy/crawler.py index b8a2855f0..32cde0300 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -159,7 +159,7 @@ class Crawler: self._apply_settings() self._update_root_log_handler() self.engine = self._create_engine() - yield deferred_from_coro(self.engine.open_spider_async(self.spider)) + yield deferred_from_coro(self.engine.open_spider_async()) yield deferred_from_coro(self.engine.start_async()) except Exception: self.crawling = False @@ -195,7 +195,7 @@ class Crawler: self._apply_settings() self._update_root_log_handler() self.engine = self._create_engine() - await self.engine.open_spider_async(self.spider) + await self.engine.open_spider_async() await self.engine.start_async() except Exception: self.crawling = False diff --git a/scrapy/middleware.py b/scrapy/middleware.py index a4d7af7b6..3ca81bcb1 100644 --- a/scrapy/middleware.py +++ b/scrapy/middleware.py @@ -10,6 +10,7 @@ from typing import TYPE_CHECKING, Any, TypeVar, cast from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning from scrapy.utils.defer import process_chain, process_parallel from scrapy.utils.misc import build_from_crawler, load_object +from scrapy.utils.python import global_object_name if TYPE_CHECKING: from collections.abc import Callable, Iterable @@ -37,9 +38,19 @@ class MiddlewareManager(ABC): """Base class for implementing middleware managers""" component_name: str + _compat_spider: Spider | None = None - def __init__(self, *middlewares: Any) -> None: - self.middlewares = middlewares + def __init__(self, *middlewares: Any, crawler: Crawler | None = None) -> None: + self.crawler: Crawler | None = crawler + if crawler is None: + warnings.warn( + f"MiddlewareManager.__init__() was called without the crawler argument" + f" when creating {global_object_name(self.__class__)}." + f" This is deprecated and the argument will be required in future Scrapy versions.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + self.middlewares: tuple[Any, ...] = middlewares # Only process_spider_output and process_spider_exception can be None. # Only process_spider_output can be a tuple, and only until _async compatibility methods are removed. self.methods: dict[str, deque[Callable | tuple[Callable, Callable] | None]] = ( @@ -48,6 +59,43 @@ class MiddlewareManager(ABC): for mw in middlewares: self._add_middleware(mw) + @property + def _spider(self) -> Spider: + if self.crawler is not None: + if self.crawler.spider is None: + raise ValueError( + f"{type(self).__name__} needs to access self.crawler.spider but it is None." + ) + return self.crawler.spider + if self._compat_spider is not None: + return self._compat_spider + raise ValueError(f"{type(self).__name__} has no known Spider instance.") + + def _set_compat_spider(self, spider: Spider | None) -> None: + if spider is None or self.crawler is not None: + return + # printing a deprecation warning is the caller's responsibility + if self._compat_spider is None: + self._compat_spider = spider + elif self._compat_spider is not spider: + raise RuntimeError( + f"Different instances of Spider were passed to {type(self).__name__}:" + f" {self._compat_spider} and {spider}" + ) + + def _warn_spider_arg(self, method_name: str) -> None: + if self.crawler: + msg = ( + f"Passing a spider argument to {type(self).__name__}.{method_name}() is deprecated" + " and the passed value is ignored." + ) + else: + msg = ( + f"Passing a spider argument to {type(self).__name__}.{method_name}() is deprecated," + f" {type(self).__name__} should be instantiated with a Crawler instance instead." + ) + warnings.warn(msg, category=ScrapyDeprecationWarning, stacklevel=3) + @classmethod @abstractmethod def _get_mwlist_from_settings(cls, settings: Settings) -> list[Any]: @@ -108,7 +156,7 @@ class MiddlewareManager(ABC): }, extra={"crawler": crawler}, ) - return cls(*middlewares) + return cls(*middlewares, crawler=crawler) def _add_middleware(self, mw: Any) -> None: if hasattr(mw, "open_spider"): @@ -130,8 +178,14 @@ class MiddlewareManager(ABC): ) return process_chain(methods, obj, *args) - def open_spider(self, spider: Spider) -> Deferred[list[None]]: - return self._process_parallel("open_spider", spider) + def open_spider(self, spider: Spider | None = None) -> Deferred[list[None]]: + if spider: + self._warn_spider_arg("open_spider") + self._set_compat_spider(spider) + return self._process_parallel("open_spider", self._spider) - def close_spider(self, spider: Spider) -> Deferred[list[None]]: - return self._process_parallel("close_spider", spider) + def close_spider(self, spider: Spider | None = None) -> Deferred[list[None]]: + if spider: + self._warn_spider_arg("close_spider") + self._set_compat_spider(spider) + return self._process_parallel("close_spider", self._spider) diff --git a/scrapy/pipelines/__init__.py b/scrapy/pipelines/__init__.py index 01f8bd2c8..e4fd4b61f 100644 --- a/scrapy/pipelines/__init__.py +++ b/scrapy/pipelines/__init__.py @@ -33,5 +33,8 @@ class ItemPipelineManager(MiddlewareManager): deferred_f_from_coro_f(pipe.process_item) ) - def process_item(self, item: Any, spider: Spider) -> Deferred[Any]: - return self._process_chain("process_item", item, spider) + def process_item(self, item: Any, spider: Spider | None = None) -> Deferred[Any]: + if spider: + self._warn_spider_arg("process_item") + self._set_compat_spider(spider) + return self._process_chain("process_item", item, self._spider) diff --git a/scrapy/shell.py b/scrapy/shell.py index 77f9ab2c1..39366312f 100644 --- a/scrapy/shell.py +++ b/scrapy/shell.py @@ -126,7 +126,7 @@ class Shell: self.crawler.spider = spider assert self.crawler.engine - await self.crawler.engine.open_spider_async(spider, close_if_idle=False) + await self.crawler.engine.open_spider_async(close_if_idle=False) self.crawler.engine._start_request_processing() self.spider = spider diff --git a/scrapy/utils/deprecate.py b/scrapy/utils/deprecate.py index 20d03cae6..a51695f70 100644 --- a/scrapy/utils/deprecate.py +++ b/scrapy/utils/deprecate.py @@ -4,9 +4,13 @@ from __future__ import annotations import inspect import warnings -from typing import Any, overload +from typing import TYPE_CHECKING, Any, overload from scrapy.exceptions import ScrapyDeprecationWarning +from scrapy.utils.python import get_func_args_dict + +if TYPE_CHECKING: + from collections.abc import Callable def attribute(obj: Any, oldattr: str, newattr: str, version: str = "0.12") -> None: @@ -191,3 +195,25 @@ def method_is_overridden(subclass: type, base_class: type, method_name: str) -> base_method = getattr(base_class, method_name) sub_method = getattr(subclass, method_name) return base_method.__code__ is not sub_method.__code__ + + +def argument_is_required(func: Callable[..., Any], arg_name: str) -> bool: + """ + Check if a function argument is required (exists and doesn't have a default value). + + .. versionadded:: VERSION + + >>> def func(a, b=1, c=None): + ... pass + >>> argument_is_required(func, 'a') + True + >>> argument_is_required(func, 'b') + False + >>> argument_is_required(func, 'c') + False + >>> argument_is_required(func, 'd') + False + """ + args = get_func_args_dict(func) + param = args.get(arg_name) + return param is not None and param.default is inspect.Parameter.empty diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py index c859fbc2a..69ab97298 100644 --- a/scrapy/utils/python.py +++ b/scrapy/utils/python.py @@ -213,35 +213,46 @@ def binary_is_text(data: bytes) -> bool: return all(c not in _BINARYCHARS for c in data) -def get_func_args(func: Callable[..., Any], stripself: bool = False) -> list[str]: - """Return the argument name list of a callable object""" +def get_func_args_dict( + func: Callable[..., Any], stripself: bool = False +) -> Mapping[str, inspect.Parameter]: + """Return the argument dict of a callable object. + + .. versionadded:: VERSION + """ if not callable(func): raise TypeError(f"func must be callable, got '{type(func).__name__}'") - args: list[str] = [] + args: Mapping[str, inspect.Parameter] try: sig = inspect.signature(func) except ValueError: - return args + return {} if isinstance(func, partial): partial_args = func.args partial_kw = func.keywords + args = {} for name, param in sig.parameters.items(): - if param.name in partial_args: + if name in partial_args: continue - if partial_kw and param.name in partial_kw: + if partial_kw and name in partial_kw: continue - args.append(name) + args[name] = param else: - args = list(sig.parameters) + args = sig.parameters - if stripself and args and args[0] == "self": - args = args[1:] + if stripself and args and "self" in args: + args = {k: v for k, v in args.items() if k != "self"} return args +def get_func_args(func: Callable[..., Any], stripself: bool = False) -> list[str]: + """Return the argument name list of a callable object""" + return list(get_func_args_dict(func, stripself=stripself)) + + def get_spec(func: Callable[..., Any]) -> tuple[list[str], dict[str, Any]]: """Returns (args, kwargs) tuple for a function >>> import re diff --git a/tests/test_core_downloader.py b/tests/test_core_downloader.py index 9d33f4280..a84bb9d8d 100644 --- a/tests/test_core_downloader.py +++ b/tests/test_core_downloader.py @@ -10,16 +10,18 @@ from twisted.web import server, static from twisted.web.client import Agent, BrowserLikePolicyForHTTPS, readBody from twisted.web.client import Response as TxResponse -from scrapy.core.downloader import Slot +from scrapy.core.downloader import Downloader, Slot from scrapy.core.downloader.contextfactory import ( ScrapyClientContextFactory, load_context_factory_from_settings, ) from scrapy.core.downloader.handlers.http11 import _RequestBodyProducer +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.settings import Settings from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future from scrapy.utils.misc import build_from_crawler from scrapy.utils.python import to_bytes +from scrapy.utils.spider import DefaultSpider from scrapy.utils.test import get_crawler from tests.mockserver.http_resources import PayloadResource from tests.mockserver.utils import ssl_context_factory @@ -172,3 +174,17 @@ class TestContextFactoryTLSMethod(TestContextFactoryBase): client_context_factory = ScrapyClientContextFactory(OpenSSL.SSL.TLSv1_2_METHOD) assert client_context_factory._ssl_method == OpenSSL.SSL.TLSv1_2_METHOD await self._assert_factory_works(server_url, client_context_factory) + + +@deferred_f_from_coro_f +async def test_fetch_deprecated_spider_arg(): + class CustomDownloader(Downloader): + def fetch(self, request, spider): # pylint: disable=signature-differs + return super().fetch(request, spider) + + crawler = get_crawler(DefaultSpider, {"DOWNLOADER": CustomDownloader}) + with pytest.warns( + ScrapyDeprecationWarning, + match=r"The fetch\(\) method of .+\.CustomDownloader requires a spider argument", + ): + await maybe_deferred_to_future(crawler.crawl()) diff --git a/tests/test_downloadermiddleware.py b/tests/test_downloadermiddleware.py index cfab0966a..6a0341414 100644 --- a/tests/test_downloadermiddleware.py +++ b/tests/test_downloadermiddleware.py @@ -3,38 +3,41 @@ from __future__ import annotations import asyncio from contextlib import asynccontextmanager from gzip import BadGzipFile +from typing import TYPE_CHECKING from unittest import mock import pytest from twisted.internet.defer import Deferred, succeed from scrapy.core.downloader.middleware import DownloaderMiddlewareManager -from scrapy.exceptions import _InvalidOutput +from scrapy.exceptions import ScrapyDeprecationWarning, _InvalidOutput from scrapy.http import Request, Response from scrapy.spiders import Spider from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future from scrapy.utils.python import to_bytes from scrapy.utils.test import get_crawler, get_from_asyncio_queue +if TYPE_CHECKING: + from collections.abc import AsyncGenerator + class TestManagerBase: settings_dict = None # should be a fixture but async fixtures that use Futures are problematic with pytest-twisted @asynccontextmanager - async def get_mwman_and_spider(self): + async def get_mwman(self) -> AsyncGenerator[DownloaderMiddlewareManager]: crawler = get_crawler(Spider, self.settings_dict) - spider = crawler._create_spider("foo") + crawler.spider = crawler._create_spider("foo") mwman = DownloaderMiddlewareManager.from_crawler(crawler) crawler.engine = crawler._create_engine() - await crawler.engine.open_spider_async(spider) - yield mwman, spider - await maybe_deferred_to_future(crawler.engine.close_spider(spider)) + await crawler.engine.open_spider_async() + yield mwman + await maybe_deferred_to_future(crawler.engine.close_spider(crawler.spider)) @staticmethod async def _download( mwman: DownloaderMiddlewareManager, - spider: Spider, request: Request, response: Response | None = None, ) -> Response | Request: @@ -45,12 +48,10 @@ class TestManagerBase: if not response: response = Response(request.url) - def download_func(request: Request, spider: Spider) -> Deferred[Response]: + def download_func(request: Request) -> Deferred[Response]: return succeed(response) - return await maybe_deferred_to_future( - mwman.download(download_func, request, spider) - ) + return await maybe_deferred_to_future(mwman.download(download_func, request)) class TestDefaults(TestManagerBase): @@ -60,8 +61,8 @@ class TestDefaults(TestManagerBase): async def test_request_response(self): req = Request("http://example.com/index.html") resp = Response(req.url, status=200) - async with self.get_mwman_and_spider() as (mwman, spider): - ret = await self._download(mwman, spider, req, resp) + async with self.get_mwman() as mwman: + ret = await self._download(mwman, req, resp) assert isinstance(ret, Response), "Non-response returned" @deferred_f_from_coro_f @@ -90,8 +91,8 @@ class TestDefaults(TestManagerBase): "Location": "http://example.com/login", }, ) - async with self.get_mwman_and_spider() as (mwman, spider): - ret = await self._download(mwman, spider, req, resp) + async with self.get_mwman() as mwman: + ret = await self._download(mwman, req, resp) assert isinstance(ret, Request), f"Not redirected: {ret!r}" assert to_bytes(ret.url) == resp.headers["Location"], ( "Not redirected to location header" @@ -113,8 +114,8 @@ class TestDefaults(TestManagerBase): }, ) with pytest.raises(BadGzipFile): - async with self.get_mwman_and_spider() as (mwman, spider): - await self._download(mwman, spider, req, resp) + async with self.get_mwman() as mwman: + await self._download(mwman, req, resp) class TestResponseFromProcessRequest(TestManagerBase): @@ -130,11 +131,9 @@ class TestResponseFromProcessRequest(TestManagerBase): def process_request(self, request, spider): return resp - async with self.get_mwman_and_spider() as (mwman, spider): + async with self.get_mwman() as mwman: mwman._add_middleware(ResponseMiddleware()) - result = await maybe_deferred_to_future( - mwman.download(download_func, req, spider) - ) + result = await maybe_deferred_to_future(mwman.download(download_func, req)) assert result is resp assert not download_func.called @@ -148,7 +147,7 @@ class TestResponseFromProcessException(TestManagerBase): resp = Response("http://example.com/index.html") calls = [] - def download_func(request, spider): + def download_func(request): raise ValueError("test") class ResponseMiddleware: @@ -160,11 +159,9 @@ class TestResponseFromProcessException(TestManagerBase): calls.append("process_exception") return resp - async with self.get_mwman_and_spider() as (mwman, spider): + async with self.get_mwman() as mwman: mwman._add_middleware(ResponseMiddleware()) - result = await maybe_deferred_to_future( - mwman.download(download_func, req, spider) - ) + result = await maybe_deferred_to_future(mwman.download(download_func, req)) assert result is resp assert calls == [ "process_exception", @@ -182,10 +179,10 @@ class TestInvalidOutput(TestManagerBase): def process_request(self, request, spider): return 1 - async with self.get_mwman_and_spider() as (mwman, spider): + async with self.get_mwman() as mwman: mwman._add_middleware(InvalidProcessRequestMiddleware()) with pytest.raises(_InvalidOutput): - await self._download(mwman, spider, req) + await self._download(mwman, req) @deferred_f_from_coro_f async def test_invalid_process_response(self): @@ -196,10 +193,10 @@ class TestInvalidOutput(TestManagerBase): def process_response(self, request, response, spider): return 1 - async with self.get_mwman_and_spider() as (mwman, spider): + async with self.get_mwman() as mwman: mwman._add_middleware(InvalidProcessResponseMiddleware()) with pytest.raises(_InvalidOutput): - await self._download(mwman, spider, req) + await self._download(mwman, req) @deferred_f_from_coro_f async def test_invalid_process_exception(self): @@ -213,10 +210,10 @@ class TestInvalidOutput(TestManagerBase): def process_exception(self, request, exception, spider): return 1 - async with self.get_mwman_and_spider() as (mwman, spider): + async with self.get_mwman() as mwman: mwman._add_middleware(InvalidProcessExceptionMiddleware()) with pytest.raises(_InvalidOutput): - await self._download(mwman, spider, req) + await self._download(mwman, req) class TestMiddlewareUsingDeferreds(TestManagerBase): @@ -238,11 +235,9 @@ class TestMiddlewareUsingDeferreds(TestManagerBase): d.callback(resp) return d - async with self.get_mwman_and_spider() as (mwman, spider): + async with self.get_mwman() as mwman: mwman._add_middleware(DeferredMiddleware()) - result = await maybe_deferred_to_future( - mwman.download(download_func, req, spider) - ) + result = await maybe_deferred_to_future(mwman.download(download_func, req)) assert result is resp assert not download_func.called @@ -261,11 +256,9 @@ class TestMiddlewareUsingCoro(TestManagerBase): await succeed(42) return resp - async with self.get_mwman_and_spider() as (mwman, spider): + async with self.get_mwman() as mwman: mwman._add_middleware(CoroMiddleware()) - result = await maybe_deferred_to_future( - mwman.download(download_func, req, spider) - ) + result = await maybe_deferred_to_future(mwman.download(download_func, req)) assert result is resp assert not download_func.called @@ -281,10 +274,44 @@ class TestMiddlewareUsingCoro(TestManagerBase): await asyncio.sleep(0.1) return await get_from_asyncio_queue(resp) - async with self.get_mwman_and_spider() as (mwman, spider): + async with self.get_mwman() as mwman: mwman._add_middleware(CoroMiddleware()) - result = await maybe_deferred_to_future( - mwman.download(download_func, req, spider) - ) + result = await maybe_deferred_to_future(mwman.download(download_func, req)) assert result is resp assert not download_func.called + + +class TestDownloadDeprecated(TestManagerBase): + @deferred_f_from_coro_f + async def test_download_func_spider_arg(self): + req = Request("http://example.com/index.html") + resp = Response(req.url, status=200) + + def download_func(request: Request, spider: Spider) -> Deferred[Response]: + return succeed(resp) + + async with self.get_mwman() as mwman: + with pytest.warns( + ScrapyDeprecationWarning, + match="The spider argument of download_func is deprecated", + ): + ret = await maybe_deferred_to_future(mwman.download(download_func, req)) + assert isinstance(ret, Response) + + @deferred_f_from_coro_f + async def test_mwman_download_spider_arg(self): + req = Request("http://example.com/index.html") + resp = Response(req.url, status=200) + + def download_func(request: Request) -> Deferred[Response]: + return succeed(resp) + + async with self.get_mwman() as mwman: + with pytest.warns( + ScrapyDeprecationWarning, + match=r"Passing a spider argument to DownloaderMiddlewareManager.download\(\) is deprecated", + ): + ret = await maybe_deferred_to_future( + mwman.download(download_func, req, mwman.crawler.spider) + ) + assert isinstance(ret, Response) diff --git a/tests/test_downloaderslotssettings.py b/tests/test_downloaderslotssettings.py index 55a67d6d2..960fcb30a 100644 --- a/tests/test_downloaderslotssettings.py +++ b/tests/test_downloaderslotssettings.py @@ -5,6 +5,7 @@ from twisted.internet.defer import inlineCallbacks from scrapy import Request from scrapy.core.downloader import Downloader, Slot from scrapy.crawler import CrawlerRunner +from scrapy.utils.spider import DefaultSpider from scrapy.utils.test import get_crawler from tests.mockserver.http import MockServer from tests.spiders import MetaSpider @@ -89,11 +90,12 @@ def test_params(): "example.com": params, }, } - crawler = get_crawler(settings_dict=settings) + crawler = get_crawler(DefaultSpider, settings_dict=settings) + crawler.spider = crawler._create_spider() downloader = Downloader(crawler) downloader._slot_gc_loop.stop() # Prevent an unclean reactor. request = Request("https://example.com") - _, actual = downloader._get_slot(request, spider=None) + _, actual = downloader._get_slot(request) expected = Slot(**params) for param in params: assert getattr(expected, param) == getattr(actual, param), ( diff --git a/tests/test_engine.py b/tests/test_engine.py index a742fd365..5e0518ded 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -441,8 +441,10 @@ class TestEngine(TestEngineBase): @inlineCallbacks def test_start_already_running_exception(self): - e = ExecutionEngine(get_crawler(DefaultSpider), lambda _: None) - yield deferred_from_coro(e.open_spider_async(DefaultSpider())) + crawler = get_crawler(DefaultSpider) + crawler.spider = crawler._create_spider() + e = ExecutionEngine(crawler, lambda _: None) + yield deferred_from_coro(e.open_spider_async()) _schedule_coro(e.start_async()) with pytest.raises(RuntimeError, match="Engine already running"): yield deferred_from_coro(e.start_async()) @@ -451,8 +453,10 @@ class TestEngine(TestEngineBase): @pytest.mark.only_asyncio @deferred_f_from_coro_f async def test_start_already_running_exception_asyncio(self): - e = ExecutionEngine(get_crawler(DefaultSpider), lambda _: None) - await e.open_spider_async(DefaultSpider()) + crawler = get_crawler(DefaultSpider) + crawler.spider = crawler._create_spider() + e = ExecutionEngine(crawler, lambda _: None) + await e.open_spider_async() with pytest.raises(RuntimeError, match="Engine already running"): await asyncio.gather(e.start_async(), e.start_async()) await deferred_to_future(e.stop()) @@ -537,12 +541,11 @@ class TestEngineDownloadAsync: assert result == response engine._slot.add_request.assert_called_once_with(request) engine._slot.remove_request.assert_called_once_with(request) - engine.downloader.fetch.assert_called_once_with(request, engine.spider) + engine.downloader.fetch.assert_called_once_with(request) @deferred_f_from_coro_f async def test_download_async_redirect(self, engine): """Test async download with a redirect request.""" - # Arrange original_request = Request("http://example.com") redirect_request = Request("http://example.com/redirect") final_response = Response("http://example.com/redirect", body=b"redirected") diff --git a/tests/test_middleware.py b/tests/test_middleware.py index 3dd1334b2..b6a07f872 100644 --- a/tests/test_middleware.py +++ b/tests/test_middleware.py @@ -1,7 +1,18 @@ -from scrapy.exceptions import NotConfigured +from __future__ import annotations + +from typing import TYPE_CHECKING + +import pytest + +from scrapy import Spider +from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning from scrapy.middleware import MiddlewareManager +from scrapy.utils.spider import DefaultSpider from scrapy.utils.test import get_crawler +if TYPE_CHECKING: + from scrapy.crawler import Crawler + class M1: def open_spider(self, spider): @@ -51,27 +62,116 @@ class MyMiddlewareManager(MiddlewareManager): self.methods["process"].append(mw.process) -class TestMiddlewareManager: - def test_init(self): - m1, m2, m3 = M1(), M2(), M3() +@pytest.fixture +def crawler() -> Crawler: + return get_crawler(Spider) + + +def test_init(crawler: Crawler) -> None: + m1, m2, m3 = M1(), M2(), M3() + mwman = MyMiddlewareManager(m1, m2, m3, crawler=crawler) + assert list(mwman.methods["open_spider"]) == [m1.open_spider, m2.open_spider] + assert list(mwman.methods["close_spider"]) == [m2.close_spider, m1.close_spider] + assert list(mwman.methods["process"]) == [m1.process, m3.process] + assert mwman.crawler == crawler + + +def test_methods(crawler: Crawler) -> None: + mwman = MyMiddlewareManager(M1(), M2(), M3(), crawler=crawler) + assert [x.__self__.__class__ for x in mwman.methods["open_spider"]] == [M1, M2] # type: ignore[union-attr] + assert [x.__self__.__class__ for x in mwman.methods["close_spider"]] == [M2, M1] # type: ignore[union-attr] + assert [x.__self__.__class__ for x in mwman.methods["process"]] == [M1, M3] # type: ignore[union-attr] + + +def test_enabled(crawler: Crawler) -> None: + m1, m2, m3 = M1(), M2(), M3() + mwman = MyMiddlewareManager(m1, m2, m3, crawler=crawler) + assert mwman.middlewares == (m1, m2, m3) + + +def test_enabled_from_settings(crawler: Crawler) -> None: + crawler = get_crawler() + mwman = MyMiddlewareManager.from_crawler(crawler) + classes = [x.__class__ for x in mwman.middlewares] + assert classes == [M1, M3] + assert mwman.crawler == crawler + + +def test_no_crawler() -> None: + m1, m2, m3 = M1(), M2(), M3() + with pytest.warns( + ScrapyDeprecationWarning, match="was called without the crawler argument" + ): mwman = MyMiddlewareManager(m1, m2, m3) - assert list(mwman.methods["open_spider"]) == [m1.open_spider, m2.open_spider] - assert list(mwman.methods["close_spider"]) == [m2.close_spider, m1.close_spider] - assert list(mwman.methods["process"]) == [m1.process, m3.process] + assert mwman.middlewares == (m1, m2, m3) + assert mwman.crawler is None - def test_methods(self): - mwman = MyMiddlewareManager(M1(), M2(), M3()) - assert [x.__self__.__class__ for x in mwman.methods["open_spider"]] == [M1, M2] - assert [x.__self__.__class__ for x in mwman.methods["close_spider"]] == [M2, M1] - assert [x.__self__.__class__ for x in mwman.methods["process"]] == [M1, M3] - def test_enabled(self): - m1, m2, m3 = M1(), M2(), M3() - mwman = MyMiddlewareManager(m1, m2, m3) - assert mwman.middlewares == (m1, m2, m3) +def test_deprecated_spider_arg_no_crawler_spider(crawler: Crawler) -> None: + """Crawler is provided, but doesn't have a spider. The instance passed to the method is + ignored and raises a warning.""" + mwman = MyMiddlewareManager(crawler=crawler) + with ( + pytest.warns( + ScrapyDeprecationWarning, + match=r"Passing a spider argument to MyMiddlewareManager.open_spider\(\) is deprecated", + ), + pytest.raises( + ValueError, + match="MyMiddlewareManager needs to access self.crawler.spider but it is None", + ), + ): + mwman.open_spider(DefaultSpider()) - def test_enabled_from_settings(self): - crawler = get_crawler() - mwman = MyMiddlewareManager.from_crawler(crawler) - classes = [x.__class__ for x in mwman.middlewares] - assert classes == [M1, M3] + +def test_deprecated_spider_arg_with_crawler(crawler: Crawler) -> None: + """Crawler is provided and has a spider, works. The instance passed to the method is ignored, + even if mismatched, but raises a warning.""" + mwman = MyMiddlewareManager(crawler=crawler) + crawler.spider = crawler._create_spider("foo") + with pytest.warns( + ScrapyDeprecationWarning, + match=r"Passing a spider argument to MyMiddlewareManager.open_spider\(\) is deprecated", + ): + mwman.open_spider(DefaultSpider()) + with pytest.warns( + ScrapyDeprecationWarning, + match=r"Passing a spider argument to MyMiddlewareManager.close_spider\(\) is deprecated", + ): + mwman.close_spider(DefaultSpider()) + + +def test_deprecated_spider_arg_without_crawler() -> None: + """The first instance passed to the method is used, with a warning. Mismatched ones raise an error.""" + with pytest.warns( + ScrapyDeprecationWarning, + match="was called without the crawler argument", + ): + mwman = MyMiddlewareManager() + with pytest.warns( + ScrapyDeprecationWarning, + match=r"Passing a spider argument to MyMiddlewareManager.open_spider\(\) is deprecated", + ): + mwman.open_spider(DefaultSpider()) + with ( + pytest.warns( + ScrapyDeprecationWarning, + match=r"Passing a spider argument to MyMiddlewareManager.close_spider\(\) is deprecated", + ), + pytest.raises(RuntimeError, match="Different instances of Spider were passed"), + ): + mwman.close_spider(DefaultSpider()) + + +def test_no_spider_arg_without_crawler() -> None: + """If no crawler and no spider arg, raise an error.""" + with pytest.warns( + ScrapyDeprecationWarning, + match="was called without the crawler argument", + ): + mwman = MyMiddlewareManager() + with pytest.raises( + ValueError, + match="has no known Spider instance", + ): + mwman.open_spider() diff --git a/tests/test_pipelines.py b/tests/test_pipelines.py index cba6a713c..2b57deea5 100644 --- a/tests/test_pipelines.py +++ b/tests/test_pipelines.py @@ -4,8 +4,15 @@ import pytest from twisted.internet.defer import Deferred, inlineCallbacks from scrapy import Request, Spider, signals +from scrapy.exceptions import ScrapyDeprecationWarning +from scrapy.pipelines import ItemPipelineManager from scrapy.utils.asyncio import call_later -from scrapy.utils.defer import deferred_to_future, maybe_deferred_to_future +from scrapy.utils.defer import ( + deferred_f_from_coro_f, + deferred_to_future, + maybe_deferred_to_future, +) +from scrapy.utils.spider import DefaultSpider from scrapy.utils.test import get_crawler, get_from_asyncio_queue from tests.mockserver.http import MockServer @@ -127,3 +134,68 @@ class TestPipeline: crawler = self._create_crawler(AsyncDefNotAsyncioPipeline) yield crawler.crawl(mockserver=self.mockserver) assert len(self.items) == 1 + + +class TestCustomPipelineManager: + def test_deprecated_process_item_spider_arg(self) -> None: + class CustomPipelineManager(ItemPipelineManager): + def process_item(self, item, spider): # pylint: disable=signature-differs + return super().process_item(item, spider) + + crawler = get_crawler(DefaultSpider) + crawler.spider = crawler._create_spider() + itemproc = CustomPipelineManager.from_crawler(crawler) + with pytest.warns( + ScrapyDeprecationWarning, + match=r"Passing a spider argument to CustomPipelineManager.process_item\(\) is deprecated", + ): + itemproc.process_item({}, crawler.spider) + + @deferred_f_from_coro_f + async def test_deprecated_spider_arg_integration( + self, mockserver: MockServer + ) -> None: + class CustomPipelineManager(ItemPipelineManager): + def open_spider(self, spider): # pylint: disable=signature-differs + return super().open_spider(spider) + + def process_item(self, item, spider): # pylint: disable=signature-differs + return super().process_item(item, spider) + + items = [] + + def _on_item_scraped(item): + assert isinstance(item, dict) + assert item.get("pipeline_passed") + items.append(item) + + crawler = get_crawler( + ItemSpider, + { + "ITEM_PROCESSOR": CustomPipelineManager, + "ITEM_PIPELINES": {SimplePipeline: 1}, + }, + ) + crawler.spider = crawler._create_spider() + crawler.signals.connect(_on_item_scraped, signals.item_scraped) + with ( + pytest.warns( + ScrapyDeprecationWarning, + match=r"The open_spider\(\) method of .+\.CustomPipelineManager requires a spider argument", + ), + pytest.warns( + ScrapyDeprecationWarning, + match=r"The process_item\(\) method of .+\.CustomPipelineManager requires a spider argument", + ), + pytest.warns( + ScrapyDeprecationWarning, + match=r"Passing a spider argument to CustomPipelineManager.open_spider\(\) is deprecated", + ), + pytest.warns( + ScrapyDeprecationWarning, + match=r"Passing a spider argument to CustomPipelineManager.process_item\(\) is deprecated", + ), + ): + await maybe_deferred_to_future(crawler.crawl(mockserver=mockserver)) + + assert len(items) == 1 diff --git a/tests/test_spidermiddleware.py b/tests/test_spidermiddleware.py index f4811a741..2ef810852 100644 --- a/tests/test_spidermiddleware.py +++ b/tests/test_spidermiddleware.py @@ -21,13 +21,15 @@ from scrapy.utils.test import get_crawler if TYPE_CHECKING: from twisted.python.failure import Failure + from scrapy.crawler import Crawler + class TestSpiderMiddleware: def setup_method(self): self.request = Request("http://example.com/index.html") self.response = Response(self.request.url, request=self.request) self.crawler = get_crawler(Spider, {"SPIDER_MIDDLEWARES_BASE": {}}) - self.spider = self.crawler._create_spider("foo") + self.crawler.spider = self.crawler._create_spider("foo") self.mwman = SpiderMiddlewareManager.from_crawler(self.crawler) async def _scrape_response(self) -> Any: @@ -42,7 +44,7 @@ class TestSpiderMiddleware: return defer.succeed(it) return await self.mwman.scrape_response_async( - scrape_func, self.response, self.request, self.spider + scrape_func, self.response, self.request ) @@ -146,10 +148,10 @@ class TestBaseAsyncSpiderMiddleware(TestSpiderMiddleware): self.crawler = get_crawler( Spider, {"SPIDER_MIDDLEWARES_BASE": {}, "SPIDER_MIDDLEWARES": setting} ) - self.spider = self.crawler._create_spider("foo") + self.crawler.spider = self.crawler._create_spider("foo") self.mwman = SpiderMiddlewareManager.from_crawler(self.crawler) return await self.mwman.scrape_response_async( - self._scrape_func, self.response, self.request, self.spider + self._scrape_func, self.response, self.request ) async def _test_simple_base( @@ -367,9 +369,9 @@ class TestProcessStartSimple(TestBaseAsyncSpiderMiddleware): self.crawler = get_crawler( TestSpider, {"SPIDER_MIDDLEWARES_BASE": {}, "SPIDER_MIDDLEWARES": setting} ) - self.spider = self.crawler._create_spider() + self.crawler.spider = self.crawler._create_spider() self.mwman = SpiderMiddlewareManager.from_crawler(self.crawler) - return await self.mwman.process_start(self.spider) + return await self.mwman.process_start() @deferred_f_from_coro_f async def test_simple(self): @@ -404,8 +406,12 @@ class UniversalMiddlewareBothAsync: class TestUniversalMiddlewareManager: @pytest.fixture - def mwman(self) -> SpiderMiddlewareManager: - return SpiderMiddlewareManager() + def crawler(self) -> Crawler: + return get_crawler(Spider) + + @pytest.fixture + def mwman(self, crawler: Crawler) -> SpiderMiddlewareManager: + return SpiderMiddlewareManager.from_crawler(crawler) def test_simple_mw(self, mwman: SpiderMiddlewareManager) -> None: mw = ProcessSpiderOutputSimpleMiddleware() @@ -475,10 +481,10 @@ class TestBuiltinMiddlewareSimple(TestBaseAsyncSpiderMiddleware): ) -> Any: setting = self._construct_mw_setting(*mw_classes, start_index=start_index) self.crawler = get_crawler(Spider, {"SPIDER_MIDDLEWARES": setting}) - self.spider = self.crawler._create_spider("foo") + self.crawler.spider = self.crawler._create_spider("foo") self.mwman = SpiderMiddlewareManager.from_crawler(self.crawler) return await self.mwman.scrape_response_async( - self._scrape_func, self.response, self.request, self.spider + self._scrape_func, self.response, self.request ) @deferred_f_from_coro_f diff --git a/tests/test_utils_python.py b/tests/test_utils_python.py index 8e1105b09..cf1c6281d 100644 --- a/tests/test_utils_python.py +++ b/tests/test_utils_python.py @@ -236,7 +236,7 @@ def test_get_func_args(): assert get_func_args(partial_f2) == ["a", "c"] assert get_func_args(partial_f3) == ["c"] assert get_func_args(cal) == ["a", "b", "c"] - assert get_func_args(object) == [] + assert get_func_args(object) == [] # pylint: disable=use-implicit-booleaness-not-comparison assert get_func_args(str.split, stripself=True) == ["sep", "maxsplit"] assert get_func_args(" ".join, stripself=True) == ["iterable"]