Merge pull request #4126 from elacuesta/from_crawler_downloader_handlers

Download handlers: from_crawler factory method, take crawler in __init__

scrapy/core/downloader/handlers/__init__.py

@@ -4,17 +4,17 @@ import logging
 
 from twisted.internet import defer
 
-from scrapy.exceptions import NotSupported, NotConfigured
-from scrapy.utils.httpobj import urlparse_cached
-from scrapy.utils.misc import load_object
-from scrapy.utils.python import without_none_values
 from scrapy import signals
+from scrapy.exceptions import NotConfigured, NotSupported
+from scrapy.utils.httpobj import urlparse_cached
+from scrapy.utils.misc import create_instance, load_object
+from scrapy.utils.python import without_none_values
 
 
 logger = logging.getLogger(__name__)
 
 
-class DownloadHandlers(object):
+class DownloadHandlers:
 
     def __init__(self, crawler):
         self._crawler = crawler
@@ -49,7 +49,11 @@ class DownloadHandlers(object):
             dhcls = load_object(path)
             if skip_lazy and getattr(dhcls, 'lazy', True):
                 return None
-            dh = dhcls(self._crawler.settings)
+            dh = create_instance(
+                objcls=dhcls,
+                settings=self._crawler.settings,
+                crawler=self._crawler,
+            )
         except NotConfigured as ex:
             self._notconfigured[scheme] = str(ex)
             return None

scrapy/core/downloader/handlers/datauri.py

@@ -5,12 +5,9 @@ from scrapy.responsetypes import responsetypes
 from scrapy.utils.decorators import defers
 
 
-class DataURIDownloadHandler(object):
+class DataURIDownloadHandler:
     lazy = False
 
-    def __init__(self, settings):
-        super(DataURIDownloadHandler, self).__init__()
-
     @defers
     def download_request(self, request, spider):
         uri = parse_data_uri(request.url)

scrapy/core/downloader/handlers/file.py

@@ -1,14 +1,12 @@
 from w3lib.url import file_uri_to_path
+
 from scrapy.responsetypes import responsetypes
 from scrapy.utils.decorators import defers
 
 
-class FileDownloadHandler(object):
+class FileDownloadHandler:
     lazy = False
 
-    def __init__(self, settings):
-        pass
-
     @defers
     def download_request(self, request, spider):
         filepath = file_uri_to_path(request.url)

scrapy/core/downloader/handlers/ftp.py

@@ -33,8 +33,8 @@ from io import BytesIO
 from urllib.parse import unquote
 
 from twisted.internet import reactor
-from twisted.protocols.ftp import FTPClient, CommandFailed
-from twisted.internet.protocol import Protocol, ClientCreator
+from twisted.internet.protocol import ClientCreator, Protocol
+from twisted.protocols.ftp import CommandFailed, FTPClient
 
 from scrapy.http import Response
 from scrapy.responsetypes import responsetypes
@@ -59,10 +59,11 @@ class ReceivedDataProtocol(Protocol):
     def close(self):
         self.body.close() if self.filename else self.body.seek(0)
 
+
 _CODE_RE = re.compile(r"\d+")
 
 
-class FTPDownloadHandler(object):
+class FTPDownloadHandler:
     lazy = False
 
     CODE_MAPPING = {
@@ -75,6 +76,10 @@ class FTPDownloadHandler(object):
         self.default_password = settings['FTP_PASSWORD']
         self.passive_mode = settings['FTP_PASSIVE_MODE']
 
+    @classmethod
+    def from_crawler(cls, crawler):
+        return cls(crawler.settings)
+
     def download_request(self, request, spider):
         parsed_url = urlparse_cached(request)
         user = request.meta.get("ftp_user", self.default_user)

scrapy/core/downloader/handlers/http10.py

@@ -1,17 +1,23 @@
 """Download handlers for http and https schemes
 """
 from twisted.internet import reactor
-from scrapy.utils.misc import load_object, create_instance
+
+from scrapy.utils.misc import create_instance, load_object
 from scrapy.utils.python import to_unicode
 
 
-class HTTP10DownloadHandler(object):
+class HTTP10DownloadHandler:
     lazy = False
 
-    def __init__(self, settings):
+    def __init__(self, settings, crawler=None):
         self.HTTPClientFactory = load_object(settings['DOWNLOADER_HTTPCLIENTFACTORY'])
         self.ClientContextFactory = load_object(settings['DOWNLOADER_CLIENTCONTEXTFACTORY'])
         self._settings = settings
+        self._crawler = crawler
+
+    @classmethod
+    def from_crawler(cls, crawler):
+        return cls(crawler.settings, crawler)
 
     def download_request(self, request, spider):
         """Return a deferred for the HTTP download"""
@@ -22,7 +28,11 @@ class HTTP10DownloadHandler(object):
     def _connect(self, factory):
         host, port = to_unicode(factory.host), factory.port
         if factory.scheme == b'https':
-            client_context_factory = create_instance(self.ClientContextFactory, settings=self._settings, crawler=None)
+            client_context_factory = create_instance(
+                objcls=self.ClientContextFactory,
+                settings=self._settings,
+                crawler=self._crawler,
+            )
             return reactor.connectSSL(host, port, factory, client_context_factory)
         else:
             return reactor.connectTCP(host, port, factory)

scrapy/core/downloader/handlers/http11.py

@@ -1,37 +1,37 @@
 """Download handlers for http and https schemes"""
 
-import re
 import logging
+import re
 import warnings
 from io import BytesIO
 from time import time
 from urllib.parse import urldefrag
 
-from zope.interface import implementer
-from twisted.internet import defer, reactor, protocol
+from twisted.internet import defer, protocol, reactor
+from twisted.internet.endpoints import TCP4ClientEndpoint
+from twisted.internet.error import TimeoutError
+from twisted.web.client import Agent, HTTPConnectionPool, ResponseDone, ResponseFailed, URI
+from twisted.web.http import _DataLoss, PotentialDataLoss
 from twisted.web.http_headers import Headers as TxHeaders
 from twisted.web.iweb import IBodyProducer, UNKNOWN_LENGTH
-from twisted.internet.error import TimeoutError
-from twisted.web.http import _DataLoss, PotentialDataLoss
-from twisted.web.client import Agent, ResponseDone, HTTPConnectionPool, ResponseFailed, URI
-from twisted.internet.endpoints import TCP4ClientEndpoint
+from zope.interface import implementer
 
+from scrapy.core.downloader.tls import openssl_methods
+from scrapy.core.downloader.webclient import _parse
 from scrapy.exceptions import ScrapyDeprecationWarning
 from scrapy.http import Headers
 from scrapy.responsetypes import responsetypes
-from scrapy.core.downloader.webclient import _parse
-from scrapy.core.downloader.tls import openssl_methods
-from scrapy.utils.misc import load_object, create_instance
+from scrapy.utils.misc import create_instance, load_object
 from scrapy.utils.python import to_bytes, to_unicode
 
 
 logger = logging.getLogger(__name__)
 
 
-class HTTP11DownloadHandler(object):
+class HTTP11DownloadHandler:
     lazy = False
 
-    def __init__(self, settings):
+    def __init__(self, settings, crawler=None):
         self._pool = HTTPConnectionPool(reactor, persistent=True)
         self._pool.maxPersistentPerHost = settings.getint('CONCURRENT_REQUESTS_PER_DOMAIN')
         self._pool._factory.noisy = False
@@ -41,17 +41,17 @@ class HTTP11DownloadHandler(object):
         # try method-aware context factory
         try:
             self._contextFactory = create_instance(
-                self._contextFactoryClass,
+                objcls=self._contextFactoryClass,
                 settings=settings,
-                crawler=None,
+                crawler=crawler,
                 method=self._sslMethod,
             )
         except TypeError:
             # use context factory defaults
             self._contextFactory = create_instance(
-                self._contextFactoryClass,
+                objcls=self._contextFactoryClass,
                 settings=settings,
-                crawler=None,
+                crawler=crawler,
             )
             msg = """
  '%s' does not accept `method` argument (type OpenSSL.SSL method,\
@@ -64,6 +64,10 @@ class HTTP11DownloadHandler(object):
         self._fail_on_dataloss = settings.getbool('DOWNLOAD_FAIL_ON_DATALOSS')
         self._disconnect_timeout = 1
 
+    @classmethod
+    def from_crawler(cls, crawler):
+        return cls(crawler.settings, crawler)
+
     def download_request(self, request, spider):
         """Return a deferred for the HTTP download"""
         agent = ScrapyAgent(

scrapy/core/downloader/handlers/s3.py

@@ -1,9 +1,10 @@
 from urllib.parse import unquote
 
-from scrapy.exceptions import NotConfigured
-from scrapy.utils.httpobj import urlparse_cached
-from scrapy.utils.boto import is_botocore
 from scrapy.core.downloader.handlers.http import HTTPDownloadHandler
+from scrapy.exceptions import NotConfigured
+from scrapy.utils.boto import is_botocore
+from scrapy.utils.httpobj import urlparse_cached
+from scrapy.utils.misc import create_instance
 
 
 def _get_boto_connection():
@@ -30,11 +31,12 @@ def _get_boto_connection():
     return _S3Connection
 
 
-class S3DownloadHandler(object):
+class S3DownloadHandler:
 
-    def __init__(self, settings, aws_access_key_id=None, aws_secret_access_key=None,
+    def __init__(self, settings, *,
+                 crawler=None,
+                 aws_access_key_id=None, aws_secret_access_key=None,
                  httpdownloadhandler=HTTPDownloadHandler, **kw):
-
         if not aws_access_key_id:
             aws_access_key_id = settings['AWS_ACCESS_KEY_ID']
         if not aws_secret_access_key:
@@ -67,7 +69,16 @@ class S3DownloadHandler(object):
             except Exception as ex:
                 raise NotConfigured(str(ex))
 
-        self._download_http = httpdownloadhandler(settings).download_request
+        _http_handler = create_instance(
+            objcls=httpdownloadhandler,
+            settings=settings,
+            crawler=crawler,
+        )
+        self._download_http = _http_handler.download_request
+
+    @classmethod
+    def from_crawler(cls, crawler, **kwargs):
+        return cls(crawler.settings, crawler=crawler, **kwargs)
 
     def download_request(self, request, spider):
         p = urlparse_cached(request)

tests/test_downloader_handlers.py

@@ -1,21 +1,20 @@
+import contextlib
 import os
 import shutil
 import tempfile
 from unittest import mock
-import contextlib
 
 from testfixtures import LogCapture
-from twisted.trial import unittest
+from twisted.cred import checkers, credentials, portal
+from twisted.internet import defer, error, reactor
 from twisted.protocols.policies import WrappingFactory
 from twisted.python.filepath import FilePath
-from twisted.internet import reactor, defer, error
-from twisted.web import server, static, util, resource
+from twisted.trial import unittest
+from twisted.web import resource, server, static, util
 from twisted.web._newclient import ResponseFailed
 from twisted.web.http import _DataLoss
-from twisted.web.test.test_webclient import ForeverTakingResource, \
-        NoLengthResource, HostHeaderResource, \
-        PayloadResource
-from twisted.cred import portal, checkers, credentials
+from twisted.web.test.test_webclient import (ForeverTakingResource, HostHeaderResource,
+                                             NoLengthResource, PayloadResource)
 from w3lib.url import path_to_file_uri
 
 from scrapy.core.downloader.handlers import DownloadHandlers
@@ -26,39 +25,38 @@ from scrapy.core.downloader.handlers.http10 import HTTP10DownloadHandler
 from scrapy.core.downloader.handlers.http11 import HTTP11DownloadHandler
 from scrapy.core.downloader.handlers.s3 import S3DownloadHandler
 
-from scrapy.spiders import Spider
+from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning
 from scrapy.http import Headers, Request
 from scrapy.http.response.text import TextResponse
 from scrapy.responsetypes import responsetypes
-from scrapy.settings import Settings
-from scrapy.utils.test import get_crawler, skip_if_no_boto
+from scrapy.spiders import Spider
+from scrapy.utils.misc import create_instance
 from scrapy.utils.python import to_bytes
-from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning
+from scrapy.utils.test import get_crawler, skip_if_no_boto
 
 from tests.mockserver import MockServer, ssl_context_factory, Echo
 from tests.spiders import SingleRequestSpider
 
 
-class DummyDH(object):
+class DummyDH:
     lazy = False
 
-    def __init__(self, crawler):
-        pass
 
-
-class DummyLazyDH(object):
+class DummyLazyDH:
     # Default is lazy for backward compatibility
-
-    def __init__(self, crawler):
-        pass
+    pass
 
 
-class OffDH(object):
+class OffDH:
     lazy = False
 
     def __init__(self, crawler):
         raise NotConfigured
 
+    @classmethod
+    def from_crawler(cls, crawler):
+        return cls(crawler)
+
 
 class LoadTestCase(unittest.TestCase):
 
@@ -106,7 +104,8 @@ class FileTestCase(unittest.TestCase):
         self.tmpname = self.mktemp()
         with open(self.tmpname + '^', 'w') as f:
             f.write('0123456789')
-        self.download_request = FileDownloadHandler(Settings()).download_request
+        handler = create_instance(FileDownloadHandler, None, get_crawler())
+        self.download_request = handler.download_request
 
     def tearDown(self):
         os.unlink(self.tmpname + '^')
@@ -239,7 +238,7 @@ class HttpTestCase(unittest.TestCase):
         else:
             self.port = reactor.listenTCP(0, self.wrapper, interface=self.host)
         self.portno = self.port.getHost().port
-        self.download_handler = self.download_handler_cls(Settings())
+        self.download_handler = create_instance(self.download_handler_cls, None, get_crawler())
         self.download_request = self.download_handler.download_request
 
     @defer.inlineCallbacks
@@ -479,9 +478,8 @@ class Http11TestCase(HttpTestCase):
         return self.test_download_broken_content_allow_data_loss('broken-chunked')
 
     def test_download_broken_content_allow_data_loss_via_setting(self, url='broken'):
-        download_handler = self.download_handler_cls(Settings({
-            'DOWNLOAD_FAIL_ON_DATALOSS': False,
-        }))
+        crawler = get_crawler(settings_dict={'DOWNLOAD_FAIL_ON_DATALOSS': False})
+        download_handler = create_instance(self.download_handler_cls, None, crawler)
         request = Request(self.getURL(url))
         d = download_handler.download_request(request, Spider('foo'))
         d.addCallback(lambda r: r.flags)
@@ -499,9 +497,8 @@ class Https11TestCase(Http11TestCase):
 
     @defer.inlineCallbacks
     def test_tls_logging(self):
-        download_handler = self.download_handler_cls(Settings({
-            'DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING': True,
-        }))
+        crawler = get_crawler(settings_dict={'DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING': True})
+        download_handler = create_instance(self.download_handler_cls, None, crawler)
         try:
             with LogCapture() as log_capture:
                 request = Request(self.getURL('file'))
@@ -568,8 +565,8 @@ class Https11CustomCiphers(unittest.TestCase):
             0, self.wrapper, ssl_context_factory(self.keyfile, self.certfile, cipher_string='CAMELLIA256-SHA'),
             interface=self.host)
         self.portno = self.port.getHost().port
-        self.download_handler = self.download_handler_cls(
-            Settings({'DOWNLOADER_CLIENT_TLS_CIPHERS': 'CAMELLIA256-SHA'}))
+        crawler = get_crawler(settings_dict={'DOWNLOADER_CLIENT_TLS_CIPHERS': 'CAMELLIA256-SHA'})
+        self.download_handler = create_instance(self.download_handler_cls, None, crawler)
         self.download_request = self.download_handler.download_request
 
     @defer.inlineCallbacks
@@ -665,7 +662,7 @@ class HttpProxyTestCase(unittest.TestCase):
         wrapper = WrappingFactory(site)
         self.port = reactor.listenTCP(0, wrapper, interface='127.0.0.1')
         self.portno = self.port.getHost().port
-        self.download_handler = self.download_handler_cls(Settings())
+        self.download_handler = create_instance(self.download_handler_cls, None, get_crawler())
         self.download_request = self.download_handler.download_request
 
     @defer.inlineCallbacks
@@ -731,9 +728,7 @@ class Http11ProxyTestCase(HttpProxyTestCase):
         self.assertIn(domain, timeout.osError)
 
 
-class HttpDownloadHandlerMock(object):
-    def __init__(self, settings):
-        pass
+class HttpDownloadHandlerMock:
 
     def download_request(self, request, spider):
         return request
@@ -743,9 +738,13 @@ class S3AnonTestCase(unittest.TestCase):
 
     def setUp(self):
         skip_if_no_boto()
-        self.s3reqh = S3DownloadHandler(Settings(),
-                httpdownloadhandler=HttpDownloadHandlerMock,
-                #anon=True, # is implicit
+        crawler = get_crawler()
+        self.s3reqh = create_instance(
+            objcls=S3DownloadHandler,
+            settings=None,
+            crawler=crawler,
+            httpdownloadhandler=HttpDownloadHandlerMock,
+            # anon=True, # implicit
         )
         self.download_request = self.s3reqh.download_request
         self.spider = Spider('foo')
@@ -771,9 +770,15 @@ class S3TestCase(unittest.TestCase):
 
     def setUp(self):
         skip_if_no_boto()
-        s3reqh = S3DownloadHandler(Settings(), self.AWS_ACCESS_KEY_ID,
-                self.AWS_SECRET_ACCESS_KEY,
-                httpdownloadhandler=HttpDownloadHandlerMock)
+        crawler = get_crawler()
+        s3reqh = create_instance(
+            objcls=S3DownloadHandler,
+            settings=None,
+            crawler=crawler,
+            aws_access_key_id=self.AWS_ACCESS_KEY_ID,
+            aws_secret_access_key=self.AWS_SECRET_ACCESS_KEY,
+            httpdownloadhandler=HttpDownloadHandlerMock,
+        )
         self.download_request = s3reqh.download_request
         self.spider = Spider('foo')
 
@@ -793,7 +798,13 @@ class S3TestCase(unittest.TestCase):
 
     def test_extra_kw(self):
         try:
-            S3DownloadHandler(Settings(), extra_kw=True)
+            crawler = get_crawler()
+            create_instance(
+                objcls=S3DownloadHandler,
+                settings=None,
+                crawler=crawler,
+                extra_kw=True,
+            )
         except Exception as e:
             self.assertIsInstance(e, (TypeError, NotConfigured))
         else:
@@ -935,7 +946,8 @@ class BaseFTPTestCase(unittest.TestCase):
         self.factory = FTPFactory(portal=p)
         self.port = reactor.listenTCP(0, self.factory, interface="127.0.0.1")
         self.portNum = self.port.getHost().port
-        self.download_handler = FTPDownloadHandler(Settings())
+        crawler = get_crawler()
+        self.download_handler = create_instance(FTPDownloadHandler, crawler.settings, crawler)
         self.addCleanup(self.port.stopListening)
 
     def tearDown(self):
@@ -1049,7 +1061,8 @@ class AnonymousFTPTestCase(BaseFTPTestCase):
                                   userAnonymous=self.username)
         self.port = reactor.listenTCP(0, self.factory, interface="127.0.0.1")
         self.portNum = self.port.getHost().port
-        self.download_handler = FTPDownloadHandler(Settings())
+        crawler = get_crawler()
+        self.download_handler = create_instance(FTPDownloadHandler, crawler.settings, crawler)
         self.addCleanup(self.port.stopListening)
 
     def tearDown(self):
@@ -1059,7 +1072,8 @@ class AnonymousFTPTestCase(BaseFTPTestCase):
 class DataURITestCase(unittest.TestCase):
 
     def setUp(self):
-        self.download_handler = DataURIDownloadHandler(Settings())
+        crawler = get_crawler()
+        self.download_handler = create_instance(DataURIDownloadHandler, crawler.settings, crawler)
         self.download_request = self.download_handler.download_request
         self.spider = Spider('foo')