mirror of https://github.com/scrapy/scrapy.git
Refactor RobotsTxtMiddleware to use async def process_request(). (#6802)
This commit is contained in:
parent
15655ca834
commit
edba1ad572
|
|
@ -44,6 +44,14 @@ Backward-incompatible changes
|
|||
|
||||
- :meth:`~scrapy.spidermiddlewares.referer.ReferrerPolicy.referrer`
|
||||
|
||||
- :meth:`scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware.process_request`
|
||||
now returns a coroutine, previously it returned a
|
||||
:class:`~twisted.internet.defer.Deferred` object or ``None``. The
|
||||
``robot_parser()`` method was also changed to return a coroutine. This
|
||||
change only impacts code that subclasses
|
||||
:class:`~scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware` or
|
||||
calls its methods directly.
|
||||
|
||||
|
||||
.. _release-2.13.3:
|
||||
|
||||
|
|
|
|||
|
|
@ -9,19 +9,16 @@ from __future__ import annotations
|
|||
import logging
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from twisted.internet.defer import Deferred, maybeDeferred
|
||||
from twisted.internet.defer import Deferred
|
||||
|
||||
from scrapy.exceptions import IgnoreRequest, NotConfigured
|
||||
from scrapy.http import Request, Response
|
||||
from scrapy.http.request import NO_CALLBACK
|
||||
from scrapy.utils.defer import deferred_from_coro
|
||||
from scrapy.utils.defer import maybe_deferred_to_future
|
||||
from scrapy.utils.httpobj import urlparse_cached
|
||||
from scrapy.utils.log import failure_to_exc_info
|
||||
from scrapy.utils.misc import load_object
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from twisted.python.failure import Failure
|
||||
|
||||
# typing.Self requires Python 3.11
|
||||
from typing_extensions import Self
|
||||
|
||||
|
|
@ -54,20 +51,13 @@ class RobotsTxtMiddleware:
|
|||
def from_crawler(cls, crawler: Crawler) -> Self:
|
||||
return cls(crawler)
|
||||
|
||||
def process_request(
|
||||
self, request: Request, spider: Spider
|
||||
) -> Deferred[None] | None:
|
||||
async def process_request(self, request: Request, spider: Spider) -> None:
|
||||
if request.meta.get("dont_obey_robotstxt"):
|
||||
return None
|
||||
return
|
||||
if request.url.startswith("data:") or request.url.startswith("file:"):
|
||||
return None
|
||||
d: Deferred[RobotParser | None] = maybeDeferred(
|
||||
self.robot_parser,
|
||||
request,
|
||||
spider, # type: ignore[call-overload]
|
||||
)
|
||||
d2: Deferred[None] = d.addCallback(self.process_request_2, request, spider)
|
||||
return d2
|
||||
return
|
||||
rp = await self.robot_parser(request, spider)
|
||||
self.process_request_2(rp, request, spider)
|
||||
|
||||
def process_request_2(
|
||||
self, rp: RobotParser | None, request: Request, spider: Spider
|
||||
|
|
@ -89,9 +79,9 @@ class RobotsTxtMiddleware:
|
|||
self.crawler.stats.inc_value("robotstxt/forbidden")
|
||||
raise IgnoreRequest("Forbidden by robots.txt")
|
||||
|
||||
def robot_parser(
|
||||
async def robot_parser(
|
||||
self, request: Request, spider: Spider
|
||||
) -> RobotParser | Deferred[RobotParser | None] | None:
|
||||
) -> RobotParser | None:
|
||||
url = urlparse_cached(request)
|
||||
netloc = url.netloc
|
||||
|
||||
|
|
@ -106,35 +96,29 @@ class RobotsTxtMiddleware:
|
|||
)
|
||||
assert self.crawler.engine
|
||||
assert self.crawler.stats
|
||||
dfd = deferred_from_coro(self.crawler.engine.download_async(robotsreq))
|
||||
dfd.addCallback(self._parse_robots, netloc, spider)
|
||||
dfd.addErrback(self._logerror, robotsreq, spider)
|
||||
dfd.addErrback(self._robots_error, netloc)
|
||||
try:
|
||||
resp = await self.crawler.engine.download_async(robotsreq)
|
||||
self._parse_robots(resp, netloc)
|
||||
except Exception as e:
|
||||
self._logerror(e, robotsreq, spider)
|
||||
self._robots_error(e, netloc)
|
||||
self.crawler.stats.inc_value("robotstxt/request_count")
|
||||
|
||||
parser = self._parsers[netloc]
|
||||
if isinstance(parser, Deferred):
|
||||
d: Deferred[RobotParser | None] = Deferred()
|
||||
|
||||
def cb(result: RobotParser | None) -> RobotParser | None:
|
||||
d.callback(result)
|
||||
return result
|
||||
|
||||
parser.addCallback(cb)
|
||||
return d
|
||||
return await maybe_deferred_to_future(parser)
|
||||
return parser
|
||||
|
||||
def _logerror(self, failure: Failure, request: Request, spider: Spider) -> Failure:
|
||||
if failure.type is not IgnoreRequest:
|
||||
def _logerror(self, exc: Exception, request: Request, spider: Spider) -> None:
|
||||
if not isinstance(exc, IgnoreRequest):
|
||||
logger.error(
|
||||
"Error downloading %(request)s: %(f_exception)s",
|
||||
{"request": request, "f_exception": failure.value},
|
||||
exc_info=failure_to_exc_info(failure),
|
||||
{"request": request, "f_exception": exc},
|
||||
exc_info=True, # noqa: LOG014
|
||||
extra={"spider": spider},
|
||||
)
|
||||
return failure
|
||||
|
||||
def _parse_robots(self, response: Response, netloc: str, spider: Spider) -> None:
|
||||
def _parse_robots(self, response: Response, netloc: str) -> None:
|
||||
assert self.crawler.stats
|
||||
self.crawler.stats.inc_value("robotstxt/response_count")
|
||||
self.crawler.stats.inc_value(
|
||||
|
|
@ -146,9 +130,9 @@ class RobotsTxtMiddleware:
|
|||
self._parsers[netloc] = rp
|
||||
rp_dfd.callback(rp)
|
||||
|
||||
def _robots_error(self, failure: Failure, netloc: str) -> None:
|
||||
if failure.type is not IgnoreRequest:
|
||||
key = f"robotstxt/exception_count/{failure.type}"
|
||||
def _robots_error(self, exc: Exception, netloc: str) -> None:
|
||||
if not isinstance(exc, IgnoreRequest):
|
||||
key = f"robotstxt/exception_count/{type(exc)}"
|
||||
assert self.crawler.stats
|
||||
self.crawler.stats.inc_value(key)
|
||||
rp_dfd = self._parsers[netloc]
|
||||
|
|
|
|||
|
|
@ -1,11 +1,12 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
from typing import TYPE_CHECKING
|
||||
from unittest import mock
|
||||
|
||||
import pytest
|
||||
from twisted.internet import error
|
||||
from twisted.internet.defer import Deferred
|
||||
from twisted.internet.defer import Deferred, DeferredList
|
||||
from twisted.python import failure
|
||||
|
||||
from scrapy.downloadermiddlewares.robotstxt import RobotsTxtMiddleware
|
||||
|
|
@ -17,7 +18,7 @@ from scrapy.settings import Settings
|
|||
from scrapy.utils.asyncio import call_later
|
||||
from scrapy.utils.defer import (
|
||||
deferred_f_from_coro_f,
|
||||
ensure_awaitable,
|
||||
deferred_from_coro,
|
||||
maybe_deferred_to_future,
|
||||
)
|
||||
from tests.test_robotstxt_interface import rerp_available
|
||||
|
|
@ -78,6 +79,25 @@ Disallow: /some/randome/page.html
|
|||
Request("http://site.local/wiki/Käyttäjä:"), middleware
|
||||
)
|
||||
|
||||
@deferred_f_from_coro_f
|
||||
async def test_robotstxt_multiple_reqs(self) -> None:
|
||||
middleware = RobotsTxtMiddleware(self._get_successful_crawler())
|
||||
d1 = deferred_from_coro(
|
||||
middleware.process_request(Request("http://site.local/allowed1"), None) # type: ignore[arg-type]
|
||||
)
|
||||
d2 = deferred_from_coro(
|
||||
middleware.process_request(Request("http://site.local/allowed2"), None) # type: ignore[arg-type]
|
||||
)
|
||||
await maybe_deferred_to_future(DeferredList([d1, d2], fireOnOneErrback=True))
|
||||
|
||||
@pytest.mark.only_asyncio
|
||||
@deferred_f_from_coro_f
|
||||
async def test_robotstxt_multiple_reqs_asyncio(self) -> None:
|
||||
middleware = RobotsTxtMiddleware(self._get_successful_crawler())
|
||||
c1 = middleware.process_request(Request("http://site.local/allowed1"), None) # type: ignore[arg-type]
|
||||
c2 = middleware.process_request(Request("http://site.local/allowed2"), None) # type: ignore[arg-type]
|
||||
await asyncio.gather(c1, c2)
|
||||
|
||||
@deferred_f_from_coro_f
|
||||
async def test_robotstxt_ready_parser(self):
|
||||
middleware = RobotsTxtMiddleware(self._get_successful_crawler())
|
||||
|
|
@ -157,9 +177,7 @@ Disallow: /some/randome/page.html
|
|||
|
||||
middleware = RobotsTxtMiddleware(self.crawler)
|
||||
middleware._logerror = mock.MagicMock(side_effect=middleware._logerror)
|
||||
await maybe_deferred_to_future(
|
||||
middleware.process_request(Request("http://site.local"), None)
|
||||
)
|
||||
await middleware.process_request(Request("http://site.local"), None)
|
||||
assert middleware._logerror.called
|
||||
|
||||
@deferred_f_from_coro_f
|
||||
|
|
@ -201,32 +219,39 @@ Disallow: /some/randome/page.html
|
|||
middleware.process_request_2(rp, Request("http://site.local/allowed"), None)
|
||||
rp.allowed.assert_called_once_with("http://site.local/allowed", "Examplebot")
|
||||
|
||||
def test_robotstxt_local_file(self):
|
||||
@deferred_f_from_coro_f
|
||||
async def test_robotstxt_local_file(self):
|
||||
middleware = RobotsTxtMiddleware(self._get_emptybody_crawler())
|
||||
assert not middleware.process_request(
|
||||
middleware.process_request_2 = mock.MagicMock()
|
||||
|
||||
await middleware.process_request(
|
||||
Request("data:text/plain,Hello World data"), None
|
||||
)
|
||||
assert not middleware.process_request(
|
||||
assert not middleware.process_request_2.called
|
||||
|
||||
await middleware.process_request(
|
||||
Request("file:///tests/sample_data/test_site/nothinghere.html"), None
|
||||
)
|
||||
assert isinstance(
|
||||
middleware.process_request(Request("http://site.local/allowed"), None),
|
||||
Deferred,
|
||||
)
|
||||
assert not middleware.process_request_2.called
|
||||
|
||||
await middleware.process_request(Request("http://site.local/allowed"), None)
|
||||
assert middleware.process_request_2.called
|
||||
|
||||
async def assertNotIgnored(
|
||||
self, request: Request, middleware: RobotsTxtMiddleware
|
||||
) -> None:
|
||||
spider = None # not actually used
|
||||
result = await ensure_awaitable(middleware.process_request(request, spider)) # type: ignore[arg-type]
|
||||
assert result is None
|
||||
try:
|
||||
await middleware.process_request(request, spider) # type: ignore[arg-type]
|
||||
except IgnoreRequest:
|
||||
pytest.fail("IgnoreRequest was raised unexpectedly")
|
||||
|
||||
async def assertIgnored(
|
||||
self, request: Request, middleware: RobotsTxtMiddleware
|
||||
) -> None:
|
||||
spider = None # not actually used
|
||||
with pytest.raises(IgnoreRequest):
|
||||
await ensure_awaitable(middleware.process_request(request, spider)) # type: ignore[arg-type]
|
||||
await middleware.process_request(request, spider) # type: ignore[arg-type]
|
||||
|
||||
def assertRobotsTxtRequested(self, base_url: str) -> None:
|
||||
calls = self.crawler.engine.download_async.call_args_list
|
||||
|
|
|
|||
Loading…
Reference in New Issue