From 5d0058492c07d3f89f46ccfd2b8e33849f8bd30e Mon Sep 17 00:00:00 2001 From: Bernardas Date: Tue, 23 Aug 2016 16:54:10 +0000 Subject: [PATCH 01/17] add media pipeline settings to enable redirection and handling of certain http statuses --- scrapy/pipelines/files.py | 2 +- scrapy/pipelines/media.py | 34 +++++++++++++++++++++++++++++++--- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 843b4d3ec..4ae7e1d89 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -249,7 +249,7 @@ class FilesPipeline(MediaPipeline): resolve('FILES_RESULT_FIELD'), self.FILES_RESULT_FIELD ) - super(FilesPipeline, self).__init__(download_func=download_func) + super(FilesPipeline, self).__init__(download_func=download_func, settings=settings) @classmethod def from_settings(cls, settings): diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index 57f70499e..4177d294f 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -1,5 +1,6 @@ from __future__ import print_function +import functools import logging from collections import defaultdict from twisted.internet.defer import Deferred, DeferredList @@ -16,6 +17,7 @@ logger = logging.getLogger(__name__) class MediaPipeline(object): LOG_FAILED_RESULTS = True + ALLOW_REDIRECTS = False class SpiderInfo(object): def __init__(self, spider): @@ -24,9 +26,16 @@ class MediaPipeline(object): self.downloaded = {} self.waiting = defaultdict(list) - def __init__(self, download_func=None): + def __init__(self, download_func=None, settings=None): self.download_func = download_func - + resolve = functools.partial(self._key_for_pipe, + base_class_name="MediaPipeline") + self.allow_redirects = settings.getbool( + resolve('MEDIA_ALLOW_REDIRECTS'), self.ALLOW_REDIRECTS + ) + self.allow_httpstatus_list = settings.getlist( + resolve('MEDIA_HTTPSTATUS_LIST'), [] + ) def _key_for_pipe(self, key, base_class_name=None, settings=None): @@ -93,6 +102,25 @@ class MediaPipeline(object): ) return dfd.addBoth(lambda _: wad) # it must return wad at last + def _modify_media_request(self, request): + httpstatus_list = [] + if self.allow_httpstatus_list: + httpstatus_list = self.allow_httpstatus_list + elif self.allow_redirects: + if not httpstatus_list: + httpstatus_list = list(range(0, 300)) + list(range(400, 1000)) + else: + for i in range(300, 400): + try: + httpstatus_list.remove(i) + except ValueError: + pass + if httpstatus_list: + request.meta['handle_httpstatus_list'] = httpstatus_list + else: + request.meta['handle_httpstatus_all'] = True + return request + def _check_media_to_download(self, result, request, info): if result is not None: return result @@ -103,7 +131,7 @@ class MediaPipeline(object): callback=self.media_downloaded, callbackArgs=(request, info), errback=self.media_failed, errbackArgs=(request, info)) else: - request.meta['handle_httpstatus_all'] = True + request = self._modify_media_request(request) dfd = self.crawler.engine.download(request, info.spider) dfd.addCallbacks( callback=self.media_downloaded, callbackArgs=(request, info), From 25ed491219924b5fc98a67f64d355249a1b68f50 Mon Sep 17 00:00:00 2001 From: Bernardas Date: Tue, 23 Aug 2016 16:55:34 +0000 Subject: [PATCH 02/17] add description for media pipeline MEDIA_ALLOW_REDIRECTS and MEDIA_HTTPSTATUS_LIST settings --- docs/topics/media-pipeline.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index 82c0aaa88..a86bab4bf 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -322,6 +322,22 @@ By default, there are no size constraints, so all images are processed. .. _topics-media-pipeline-override: +Allowing redirection and handling various http statuses +------------------------------------------------------- + +.. setting:: MEDIA_ALLOW_REDIRECTS +.. setting:: MEDIA_HTTPSTATUS_LIST + +By default media pipelines ignore redirects. To allow redirecting(all 300 codes) set: + + MEDIA_ALLOW_REDIRECTS = True + +To only allow specific codes through set: + + MEDIA_HTTpSTATUS_LIST = + # example: + MEDIA_HTTPSTATUS_LIST = [303, 404] + Extending the Media Pipelines ============================= From 6a4221471610b7f3a5d44d93306294c21d550406 Mon Sep 17 00:00:00 2001 From: Bernardas Date: Tue, 23 Aug 2016 16:56:31 +0000 Subject: [PATCH 03/17] add tests for media pipeline MEDIA_ALLOW_REDIRECTS and MEDIA_HTTPSTATUS_LIST settings --- tests/test_pipeline_media.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/tests/test_pipeline_media.py b/tests/test_pipeline_media.py index f30b4fea3..41ee9962f 100644 --- a/tests/test_pipeline_media.py +++ b/tests/test_pipeline_media.py @@ -6,6 +6,7 @@ from twisted.internet import reactor from twisted.internet.defer import Deferred, inlineCallbacks from scrapy.http import Request, Response +from scrapy.settings import Settings from scrapy.spiders import Spider from scrapy.utils.request import request_fingerprint from scrapy.pipelines.media import MediaPipeline @@ -25,7 +26,8 @@ class BaseMediaPipelineTestCase(unittest.TestCase): def setUp(self): self.spider = Spider('media.com') - self.pipe = self.pipeline_class(download_func=_mocked_download_func) + self.pipe = self.pipeline_class(download_func=_mocked_download_func, + settings=Settings()) self.pipe.open_spider(self.spider) self.info = self.pipe.spiderinfo @@ -82,6 +84,21 @@ class BaseMediaPipelineTestCase(unittest.TestCase): new_item = yield self.pipe.process_item(item, self.spider) assert new_item is item + def test_modify_media_request(self): + request = Request('http://url') + assert self.pipe._modify_media_request(request).meta == {'handle_httpstatus_all': True} + + request = Request('http://url') + self.pipe.allow_httpstatus_list = list(range(100)) + assert self.pipe._modify_media_request(request).meta == {'handle_httpstatus_list': list(range(100))} + self.pipe.allow_httpstatus_list = None + + request = Request('http://url') + self.pipe.allow_redirects = True + correct = {'handle_httpstatus_list': list(range(300)) + list(range(400,1000))} + assert self.pipe._modify_media_request(request).meta == correct + self.pipe.allow_redirects = False + class MockedMediaPipeline(MediaPipeline): From 854278085494053361a8cf070237683557b642ae Mon Sep 17 00:00:00 2001 From: Bernardas Date: Tue, 23 Aug 2016 17:09:43 +0000 Subject: [PATCH 04/17] typo and clarify handling --- docs/topics/media-pipeline.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index a86bab4bf..bfc405d9e 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -332,11 +332,11 @@ By default media pipelines ignore redirects. To allow redirecting(all 300 codes) MEDIA_ALLOW_REDIRECTS = True -To only allow specific codes through set: +To only allow handling only specific codes set (default: any code): - MEDIA_HTTpSTATUS_LIST = + MEDIA_HTTPSTATUS_LIST = # example: - MEDIA_HTTPSTATUS_LIST = [303, 404] + MEDIA_HTTPSTATUS_LIST = [303, 404] # will not go through pipelines Extending the Media Pipelines ============================= From 2e052c86150502c7441c8f92cd1488de6232ec7f Mon Sep 17 00:00:00 2001 From: Bernardas Date: Tue, 23 Aug 2016 17:13:09 +0000 Subject: [PATCH 05/17] fix error when settings are not provided --- scrapy/pipelines/media.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index 4177d294f..976b5032b 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -6,6 +6,7 @@ from collections import defaultdict from twisted.internet.defer import Deferred, DeferredList from twisted.python.failure import Failure +from scrapy.settings import Settings from scrapy.utils.defer import mustbe_deferred, defer_result from scrapy.utils.request import request_fingerprint from scrapy.utils.misc import arg_to_iter @@ -28,6 +29,8 @@ class MediaPipeline(object): def __init__(self, download_func=None, settings=None): self.download_func = download_func + if isinstance(settings, dict) or settings is None: + settings = Settings(settings) resolve = functools.partial(self._key_for_pipe, base_class_name="MediaPipeline") self.allow_redirects = settings.getbool( From f0b4077f812619640df078a2485f8a460fafa58a Mon Sep 17 00:00:00 2001 From: Bernardas Date: Wed, 24 Aug 2016 08:39:30 +0000 Subject: [PATCH 06/17] expose allowed_status tuple for media pipeline --- scrapy/downloadermiddlewares/redirect.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scrapy/downloadermiddlewares/redirect.py b/scrapy/downloadermiddlewares/redirect.py index 26677e527..ae4ad8891 100644 --- a/scrapy/downloadermiddlewares/redirect.py +++ b/scrapy/downloadermiddlewares/redirect.py @@ -57,6 +57,8 @@ class RedirectMiddleware(BaseRedirectMiddleware): Handle redirection of requests based on response status and meta-refresh html tag. """ + allowed_status = (301, 302, 303, 307) + def process_response(self, request, response, spider): if (request.meta.get('dont_redirect', False) or response.status in getattr(spider, 'handle_httpstatus_list', []) or @@ -64,8 +66,7 @@ class RedirectMiddleware(BaseRedirectMiddleware): request.meta.get('handle_httpstatus_all', False)): return response - allowed_status = (301, 302, 303, 307) - if 'Location' not in response.headers or response.status not in allowed_status: + if 'Location' not in response.headers or response.status not in self.allowed_status: return response location = safe_url_string(response.headers['location']) From 3cef1cd451c8f28df8075aa4e3b65cedf5ecedfa Mon Sep 17 00:00:00 2001 From: Bernardas Date: Wed, 24 Aug 2016 08:40:46 +0000 Subject: [PATCH 07/17] adjust variable wording and redirect logic --- scrapy/pipelines/media.py | 19 +++++++++---------- tests/test_pipeline_media.py | 4 ++-- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index 976b5032b..b11e7095b 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -6,6 +6,7 @@ from collections import defaultdict from twisted.internet.defer import Deferred, DeferredList from twisted.python.failure import Failure +from scrapy.downloadermiddlewares.redirect import RedirectMiddleware from scrapy.settings import Settings from scrapy.utils.defer import mustbe_deferred, defer_result from scrapy.utils.request import request_fingerprint @@ -36,7 +37,7 @@ class MediaPipeline(object): self.allow_redirects = settings.getbool( resolve('MEDIA_ALLOW_REDIRECTS'), self.ALLOW_REDIRECTS ) - self.allow_httpstatus_list = settings.getlist( + self.handle_httpstatus_list = settings.getlist( resolve('MEDIA_HTTPSTATUS_LIST'), [] ) @@ -107,17 +108,15 @@ class MediaPipeline(object): def _modify_media_request(self, request): httpstatus_list = [] - if self.allow_httpstatus_list: - httpstatus_list = self.allow_httpstatus_list - elif self.allow_redirects: + if self.handle_httpstatus_list: + httpstatus_list = self.handle_httpstatus_list + if self.allow_redirects: if not httpstatus_list: - httpstatus_list = list(range(0, 300)) + list(range(400, 1000)) + httpstatus_list = [i for i in range(1000) + if i not in RedirectMiddleware.allowed_status] else: - for i in range(300, 400): - try: - httpstatus_list.remove(i) - except ValueError: - pass + httpstatus_list = [i for i in httpstatus_list + if i not in RedirectMiddleware.allowed_status] if httpstatus_list: request.meta['handle_httpstatus_list'] = httpstatus_list else: diff --git a/tests/test_pipeline_media.py b/tests/test_pipeline_media.py index 41ee9962f..66e98db29 100644 --- a/tests/test_pipeline_media.py +++ b/tests/test_pipeline_media.py @@ -89,9 +89,9 @@ class BaseMediaPipelineTestCase(unittest.TestCase): assert self.pipe._modify_media_request(request).meta == {'handle_httpstatus_all': True} request = Request('http://url') - self.pipe.allow_httpstatus_list = list(range(100)) + self.pipe.handle_httpstatus_list = list(range(100)) assert self.pipe._modify_media_request(request).meta == {'handle_httpstatus_list': list(range(100))} - self.pipe.allow_httpstatus_list = None + self.pipe.handle_httpstatus_list = None request = Request('http://url') self.pipe.allow_redirects = True From 11b31c9fbddd75fe7fec60152a1bec57acccc039 Mon Sep 17 00:00:00 2001 From: Bernardas Date: Wed, 24 Aug 2016 08:44:56 +0000 Subject: [PATCH 08/17] fix redirect change --- tests/test_pipeline_media.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_pipeline_media.py b/tests/test_pipeline_media.py index 66e98db29..f1b8076fd 100644 --- a/tests/test_pipeline_media.py +++ b/tests/test_pipeline_media.py @@ -5,6 +5,7 @@ from twisted.python.failure import Failure from twisted.internet import reactor from twisted.internet.defer import Deferred, inlineCallbacks +from scrapy.downloadermiddlewares.redirect import RedirectMiddleware from scrapy.http import Request, Response from scrapy.settings import Settings from scrapy.spiders import Spider @@ -95,7 +96,8 @@ class BaseMediaPipelineTestCase(unittest.TestCase): request = Request('http://url') self.pipe.allow_redirects = True - correct = {'handle_httpstatus_list': list(range(300)) + list(range(400,1000))} + correct = {'handle_httpstatus_list': [i for i in range(1000) + if i not in RedirectMiddleware.allowed_status]} assert self.pipe._modify_media_request(request).meta == correct self.pipe.allow_redirects = False From c64ebee06253ef385564c83806bd2422bff6f6d6 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Thu, 2 Mar 2017 22:40:10 +0100 Subject: [PATCH 09/17] Refactor (WIP) --- scrapy/pipelines/media.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index b11e7095b..0712101b0 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -19,7 +19,6 @@ logger = logging.getLogger(__name__) class MediaPipeline(object): LOG_FAILED_RESULTS = True - ALLOW_REDIRECTS = False class SpiderInfo(object): def __init__(self, spider): @@ -35,12 +34,23 @@ class MediaPipeline(object): resolve = functools.partial(self._key_for_pipe, base_class_name="MediaPipeline") self.allow_redirects = settings.getbool( - resolve('MEDIA_ALLOW_REDIRECTS'), self.ALLOW_REDIRECTS + resolve('MEDIA_ALLOW_REDIRECTS'), False ) self.handle_httpstatus_list = settings.getlist( resolve('MEDIA_HTTPSTATUS_LIST'), [] ) + self.httpstatus_list = [] + if self.handle_httpstatus_list: + self.httpstatus_list = self.handle_httpstatus_list + if self.allow_redirects: + if not self.httpstatus_list: + self.httpstatus_list = [i for i in range(1000) + if i not in RedirectMiddleware.allowed_status] + else: + self.httpstatus_list = [i for i in self.httpstatus_list + if i not in RedirectMiddleware.allowed_status] + def _key_for_pipe(self, key, base_class_name=None, settings=None): """ @@ -107,18 +117,8 @@ class MediaPipeline(object): return dfd.addBoth(lambda _: wad) # it must return wad at last def _modify_media_request(self, request): - httpstatus_list = [] - if self.handle_httpstatus_list: - httpstatus_list = self.handle_httpstatus_list - if self.allow_redirects: - if not httpstatus_list: - httpstatus_list = [i for i in range(1000) - if i not in RedirectMiddleware.allowed_status] - else: - httpstatus_list = [i for i in httpstatus_list - if i not in RedirectMiddleware.allowed_status] - if httpstatus_list: - request.meta['handle_httpstatus_list'] = httpstatus_list + if self.httpstatus_list: + request.meta['handle_httpstatus_list'] = self.httpstatus_list else: request.meta['handle_httpstatus_all'] = True return request From 72fbb687d7a35ddf52f82b165df0a842d0b653e0 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Fri, 3 Mar 2017 12:31:05 +0100 Subject: [PATCH 10/17] Revert "expose allowed_status tuple for media pipeline" This reverts commit 052809c73ed20b9a728a8fd7df3de5f45f2dad8d. --- scrapy/downloadermiddlewares/redirect.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/scrapy/downloadermiddlewares/redirect.py b/scrapy/downloadermiddlewares/redirect.py index ae4ad8891..26677e527 100644 --- a/scrapy/downloadermiddlewares/redirect.py +++ b/scrapy/downloadermiddlewares/redirect.py @@ -57,8 +57,6 @@ class RedirectMiddleware(BaseRedirectMiddleware): Handle redirection of requests based on response status and meta-refresh html tag. """ - allowed_status = (301, 302, 303, 307) - def process_response(self, request, response, spider): if (request.meta.get('dont_redirect', False) or response.status in getattr(spider, 'handle_httpstatus_list', []) or @@ -66,7 +64,8 @@ class RedirectMiddleware(BaseRedirectMiddleware): request.meta.get('handle_httpstatus_all', False)): return response - if 'Location' not in response.headers or response.status not in self.allowed_status: + allowed_status = (301, 302, 303, 307) + if 'Location' not in response.headers or response.status not in allowed_status: return response location = safe_url_string(response.headers['location']) From ecde166ee18935456bcf4c6fce89ab909bbffbaf Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Fri, 3 Mar 2017 15:50:34 +0100 Subject: [PATCH 11/17] Refactor without MEDIA_HTTPSTATUS_LIST setting --- docs/topics/media-pipeline.rst | 15 +++++-------- scrapy/pipelines/media.py | 25 ++++++++------------- tests/test_pipeline_media.py | 41 +++++++++++++++++++++++++--------- 3 files changed, 46 insertions(+), 35 deletions(-) diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index bfc405d9e..f258ff748 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -322,21 +322,18 @@ By default, there are no size constraints, so all images are processed. .. _topics-media-pipeline-override: -Allowing redirection and handling various http statuses -------------------------------------------------------- +Allowing redirections +--------------------- .. setting:: MEDIA_ALLOW_REDIRECTS -.. setting:: MEDIA_HTTPSTATUS_LIST -By default media pipelines ignore redirects. To allow redirecting(all 300 codes) set: +By default media pipelines ignore redirects, i.e. an HTTP redirection +to a media file URL request will mean the media download is considered failed. + +To handle media redirections, set this settings to ``True``: MEDIA_ALLOW_REDIRECTS = True -To only allow handling only specific codes set (default: any code): - - MEDIA_HTTPSTATUS_LIST = - # example: - MEDIA_HTTPSTATUS_LIST = [303, 404] # will not go through pipelines Extending the Media Pipelines ============================= diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index 0712101b0..02daf8d2c 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -6,8 +6,8 @@ from collections import defaultdict from twisted.internet.defer import Deferred, DeferredList from twisted.python.failure import Failure -from scrapy.downloadermiddlewares.redirect import RedirectMiddleware from scrapy.settings import Settings +from scrapy.utils.datatypes import SequenceExclude from scrapy.utils.defer import mustbe_deferred, defer_result from scrapy.utils.request import request_fingerprint from scrapy.utils.misc import arg_to_iter @@ -29,6 +29,7 @@ class MediaPipeline(object): def __init__(self, download_func=None, settings=None): self.download_func = download_func + if isinstance(settings, dict) or settings is None: settings = Settings(settings) resolve = functools.partial(self._key_for_pipe, @@ -36,20 +37,12 @@ class MediaPipeline(object): self.allow_redirects = settings.getbool( resolve('MEDIA_ALLOW_REDIRECTS'), False ) - self.handle_httpstatus_list = settings.getlist( - resolve('MEDIA_HTTPSTATUS_LIST'), [] - ) + self._handle_statuses(self.allow_redirects) - self.httpstatus_list = [] - if self.handle_httpstatus_list: - self.httpstatus_list = self.handle_httpstatus_list - if self.allow_redirects: - if not self.httpstatus_list: - self.httpstatus_list = [i for i in range(1000) - if i not in RedirectMiddleware.allowed_status] - else: - self.httpstatus_list = [i for i in self.httpstatus_list - if i not in RedirectMiddleware.allowed_status] + def _handle_statuses(self, allow_redirects): + self.handle_httpstatus_list = None + if allow_redirects: + self.handle_httpstatus_list = SequenceExclude(range(300, 400)) def _key_for_pipe(self, key, base_class_name=None, settings=None): @@ -117,8 +110,8 @@ class MediaPipeline(object): return dfd.addBoth(lambda _: wad) # it must return wad at last def _modify_media_request(self, request): - if self.httpstatus_list: - request.meta['handle_httpstatus_list'] = self.httpstatus_list + if self.handle_httpstatus_list: + request.meta['handle_httpstatus_list'] = self.handle_httpstatus_list else: request.meta['handle_httpstatus_all'] = True return request diff --git a/tests/test_pipeline_media.py b/tests/test_pipeline_media.py index f1b8076fd..4797956a0 100644 --- a/tests/test_pipeline_media.py +++ b/tests/test_pipeline_media.py @@ -24,11 +24,12 @@ def _mocked_download_func(request, info): class BaseMediaPipelineTestCase(unittest.TestCase): pipeline_class = MediaPipeline + settings = None def setUp(self): self.spider = Spider('media.com') self.pipe = self.pipeline_class(download_func=_mocked_download_func, - settings=Settings()) + settings=Settings(self.settings)) self.pipe.open_spider(self.spider) self.info = self.pipe.spiderinfo @@ -89,17 +90,37 @@ class BaseMediaPipelineTestCase(unittest.TestCase): request = Request('http://url') assert self.pipe._modify_media_request(request).meta == {'handle_httpstatus_all': True} - request = Request('http://url') - self.pipe.handle_httpstatus_list = list(range(100)) - assert self.pipe._modify_media_request(request).meta == {'handle_httpstatus_list': list(range(100))} - self.pipe.handle_httpstatus_list = None +class MediaPipelineAllowRedirectsTestCase(BaseMediaPipelineTestCase): + + pipeline_class = MediaPipeline + settings = { + 'MEDIA_ALLOW_REDIRECTS': True + } + + def test_modify_media_request(self): request = Request('http://url') - self.pipe.allow_redirects = True - correct = {'handle_httpstatus_list': [i for i in range(1000) - if i not in RedirectMiddleware.allowed_status]} - assert self.pipe._modify_media_request(request).meta == correct - self.pipe.allow_redirects = False + meta = self.pipe._modify_media_request(request).meta + self.assertIn('handle_httpstatus_list', meta) + for status, check in [ + (200, True), + + # These are the status codes we want + # the downloader to handle itself + (301, False), + (302, False), + (302, False), + (307, False), + (308, False), + + # we still want to get 4xx and 5xx + (400, True), + (404, True), + (500, True)]: + if check: + self.assertIn(status, meta['handle_httpstatus_list']) + else: + self.assertNotIn(status, meta['handle_httpstatus_list']) class MockedMediaPipeline(MediaPipeline): From f7e11b198efd0213bb51205b6829123476ccf2ba Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Fri, 3 Mar 2017 16:00:59 +0100 Subject: [PATCH 12/17] Cleanup --- scrapy/pipelines/media.py | 3 +-- tests/test_pipeline_media.py | 12 ++++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index 02daf8d2c..921e9e1c9 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -114,7 +114,6 @@ class MediaPipeline(object): request.meta['handle_httpstatus_list'] = self.handle_httpstatus_list else: request.meta['handle_httpstatus_all'] = True - return request def _check_media_to_download(self, result, request, info): if result is not None: @@ -126,7 +125,7 @@ class MediaPipeline(object): callback=self.media_downloaded, callbackArgs=(request, info), errback=self.media_failed, errbackArgs=(request, info)) else: - request = self._modify_media_request(request) + self._modify_media_request(request) dfd = self.crawler.engine.download(request, info.spider) dfd.addCallbacks( callback=self.media_downloaded, callbackArgs=(request, info), diff --git a/tests/test_pipeline_media.py b/tests/test_pipeline_media.py index 4797956a0..cfa2fc42b 100644 --- a/tests/test_pipeline_media.py +++ b/tests/test_pipeline_media.py @@ -5,7 +5,6 @@ from twisted.python.failure import Failure from twisted.internet import reactor from twisted.internet.defer import Deferred, inlineCallbacks -from scrapy.downloadermiddlewares.redirect import RedirectMiddleware from scrapy.http import Request, Response from scrapy.settings import Settings from scrapy.spiders import Spider @@ -88,7 +87,8 @@ class BaseMediaPipelineTestCase(unittest.TestCase): def test_modify_media_request(self): request = Request('http://url') - assert self.pipe._modify_media_request(request).meta == {'handle_httpstatus_all': True} + self.pipe._modify_media_request(request) + assert request.meta == {'handle_httpstatus_all': True} class MediaPipelineAllowRedirectsTestCase(BaseMediaPipelineTestCase): @@ -100,8 +100,8 @@ class MediaPipelineAllowRedirectsTestCase(BaseMediaPipelineTestCase): def test_modify_media_request(self): request = Request('http://url') - meta = self.pipe._modify_media_request(request).meta - self.assertIn('handle_httpstatus_list', meta) + self.pipe._modify_media_request(request) + self.assertIn('handle_httpstatus_list', request.meta) for status, check in [ (200, True), @@ -118,9 +118,9 @@ class MediaPipelineAllowRedirectsTestCase(BaseMediaPipelineTestCase): (404, True), (500, True)]: if check: - self.assertIn(status, meta['handle_httpstatus_list']) + self.assertIn(status, request.meta['handle_httpstatus_list']) else: - self.assertNotIn(status, meta['handle_httpstatus_list']) + self.assertNotIn(status, request.meta['handle_httpstatus_list']) class MockedMediaPipeline(MediaPipeline): From c3b6feca0e15309386c080d71b0b03a07d4c8635 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Fri, 3 Mar 2017 16:29:07 +0100 Subject: [PATCH 13/17] Fix setting lookup for MEDIA_ALLOWED_REDIRECTS --- scrapy/pipelines/media.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index 921e9e1c9..404bbf5bf 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -33,7 +33,8 @@ class MediaPipeline(object): if isinstance(settings, dict) or settings is None: settings = Settings(settings) resolve = functools.partial(self._key_for_pipe, - base_class_name="MediaPipeline") + base_class_name="MediaPipeline", + settings=settings) self.allow_redirects = settings.getbool( resolve('MEDIA_ALLOW_REDIRECTS'), False ) From c68f99eed843bd35224d8bf0e22b0c66bdc2122b Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Fri, 3 Mar 2017 17:03:25 +0100 Subject: [PATCH 14/17] Refactor settings tests --- tests/test_pipeline_media.py | 90 +++++++++++++++++++++++------------- 1 file changed, 58 insertions(+), 32 deletions(-) diff --git a/tests/test_pipeline_media.py b/tests/test_pipeline_media.py index cfa2fc42b..5f6a6d9e6 100644 --- a/tests/test_pipeline_media.py +++ b/tests/test_pipeline_media.py @@ -91,38 +91,6 @@ class BaseMediaPipelineTestCase(unittest.TestCase): assert request.meta == {'handle_httpstatus_all': True} -class MediaPipelineAllowRedirectsTestCase(BaseMediaPipelineTestCase): - - pipeline_class = MediaPipeline - settings = { - 'MEDIA_ALLOW_REDIRECTS': True - } - - def test_modify_media_request(self): - request = Request('http://url') - self.pipe._modify_media_request(request) - self.assertIn('handle_httpstatus_list', request.meta) - for status, check in [ - (200, True), - - # These are the status codes we want - # the downloader to handle itself - (301, False), - (302, False), - (302, False), - (307, False), - (308, False), - - # we still want to get 4xx and 5xx - (400, True), - (404, True), - (500, True)]: - if check: - self.assertIn(status, request.meta['handle_httpstatus_list']) - else: - self.assertNotIn(status, request.meta['handle_httpstatus_list']) - - class MockedMediaPipeline(MediaPipeline): def __init__(self, *args, **kwargs): @@ -289,3 +257,61 @@ class MediaPipelineTestCase(BaseMediaPipelineTestCase): self.assertEqual(new_item['results'], [(True, 'ITSME')]) self.assertEqual(self.pipe._mockcalled, \ ['get_media_requests', 'media_to_download', 'item_completed']) + + +class MediaPipelineAllowRedirectSettingsTestCase(unittest.TestCase): + + def _assert_request_no3xx(self, pipeline_class, settings): + pipe = pipeline_class(settings=Settings(settings)) + request = Request('http://url') + pipe._modify_media_request(request) + + self.assertIn('handle_httpstatus_list', request.meta) + for status, check in [ + (200, True), + + # These are the status codes we want + # the downloader to handle itself + (301, False), + (302, False), + (302, False), + (307, False), + (308, False), + + # we still want to get 4xx and 5xx + (400, True), + (404, True), + (500, True)]: + if check: + self.assertIn(status, request.meta['handle_httpstatus_list']) + else: + self.assertNotIn(status, request.meta['handle_httpstatus_list']) + + def test_standard_setting(self): + self._assert_request_no3xx( + MediaPipeline, + { + 'MEDIA_ALLOW_REDIRECTS': True + }) + + def test_subclass_standard_setting(self): + + class UserDefinedPipeline(MediaPipeline): + pass + + self._assert_request_no3xx( + UserDefinedPipeline, + { + 'MEDIA_ALLOW_REDIRECTS': True + }) + + def test_subclass_specific_setting(self): + + class UserDefinedPipeline(MediaPipeline): + pass + + self._assert_request_no3xx( + UserDefinedPipeline, + { + 'USERDEFINEDPIPELINE_MEDIA_ALLOW_REDIRECTS': True + }) From 7dcc86e61adf37fdfb77b00375040fe25e7ad832 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Fri, 10 Mar 2017 21:35:25 +0100 Subject: [PATCH 15/17] Add file listing resource + redirecting resource to MockServer --- tests/mockserver.py | 16 ++++++++++++++++ .../python-logo-master-v3-TM-flattened.png | Bin 0 -> 11155 bytes .../files/images/python-powered-h-50x65.png | Bin 0 -> 3243 bytes .../test_site/files/images/scrapy.png | Bin 0 -> 2710 bytes 4 files changed, 16 insertions(+) create mode 100644 tests/sample_data/test_site/files/images/python-logo-master-v3-TM-flattened.png create mode 100644 tests/sample_data/test_site/files/images/python-powered-h-50x65.png create mode 100644 tests/sample_data/test_site/files/images/scrapy.png diff --git a/tests/mockserver.py b/tests/mockserver.py index e611cc3ec..26ab51183 100644 --- a/tests/mockserver.py +++ b/tests/mockserver.py @@ -5,13 +5,17 @@ from subprocess import Popen, PIPE from twisted.web.server import Site, NOT_DONE_YET from twisted.web.resource import Resource +from twisted.web.static import File from twisted.web.test.test_webclient import PayloadResource from twisted.web.server import GzipEncoderFactory from twisted.web.resource import EncodingResourceWrapper +from twisted.web.util import redirectTo from twisted.internet import reactor, ssl from twisted.internet.task import deferLater + from scrapy.utils.python import to_bytes, to_unicode +from tests import tests_datadir def getarg(request, name, default=None, type=None): @@ -120,6 +124,16 @@ class Echo(LeafResource): return to_bytes(json.dumps(output)) +class RedirectTo(LeafResource): + + def render(self, request): + goto = getarg(request, b'goto', b'/') + # we force the body content, otherwise Twisted redirectTo() + # returns HTML with )mfY@00XQM~#e_i5LqDi%dgZMIQ?b+Z22sAjAis zmi_Vf!5>^-B@F{Y@Cqe#NCdx$ywpv7v9O2_{=Fee<>J}kB;6}jlUK?0TYDc*cSnbpw(L({`Zxx7`ndYC zYn!l(3JZ%T_?Hi3VX(%->L@DUDIuIY)C`>CCbyk_-T<}6^)I*C_H@QqdIY1*i>j@X(n9|*#px% zxcYZq+^^6(T%6O_J8ke+2j`4c&-7?jhPr8u#C44H+_G1AD~D4&%N_=2YL#@cHE7OK zpyP7k(<5fmZeax#@G6*@+Q+A)qM{xhK7TKgZV~f=$6MSsIlVkN0Et% zT?Hs9z6@e$he33Oa=hAq-Vxh^)RKw0WgQ9I&5eHcPSa z2n^f&Ex<7Up(}Uv?o?(o_s;+zEAc6Yn)s0qzWkEcM?pE^Ycjuc%sa<=g3WEPM~DsL zn}#<-$ug0df`6)CExWk39F2J*f0a+R3HXsLNCZo>w(frpJPX)YT+hAxu2mj9E-srs zraMl*E0(s{ZkRHR1#PyJa5f2b1tA06p-rs&qqFs6jc?mAIMdd&0JN=s3MJZ%5=r(#P zs14&N0d&CYtM&Z9aW6V<`Ij+I!6=n~qpBjRrm0uy?4AV7#U~|%6MPumooWuE&J=uj zaN;Ypa-!9&9F!I57(sTB`<~WINrn{1`g&6L@L+$hYxAx-PBH=C6l;zcYD}nnodc#+Y2{I zi)?l+dgfhM8&mAb$=Qc~xX>~+%&)USwNpvbLsPiGFb*CRM-hh(E9dHv#6&8h=Te)R zJEsx_<_x?#?5&2uM)ub!Rb9H{$=q*;hKBrha3Y(U9{)#^d$W1Z3^_N)vcA49Ge9bJ z>cHLCl!9D(NQ!aZvqDBXe(YOJEuk~o_oj1J3Q0Y#XBl|JmKW9zgtxdd?NRvkI}-KG zFyoPXv7eKhTe*9SgL7y z8+TP=+-z+jMfypQ6!kRdKpGd($6~+M;!-~1I3+eSTEX~ouER#{-+#v(3;6higzmLQ zI!gYBUM9SLCj%e2xzS)lUYh(;m)UlgdScT|YU%0uWDb7s^7oT(CS>t-a>FMueeX+# zIZY6vEWcWwCSr(9SY4e3+B4^f-l(Yj`D7tTU}&gUg*3l3FhsX*3u`dcr1#mpkk2`s9_OmVhs%LJ~HlzRG={VIOP~j&< zF9x=8(crl#Cv!X~h815%PqH)-N3_BU8~ky!Bb0Rj-Y|57Hp4u}^57Wvt9aI^NZMba z0{bguPoSKIenRj2v780vg17%XV37-L92Ov^P&WF@klFfoVc=DfH;xli7K^p3TwrYe z2zF*jwmkxbi18>K06-^jV^O|XI&LGa!L`90WaP~M+7nK z3KxW(l2EzSwY9YZlZMV@SyQ6F%&;qL9dpK#BS<`Ti_C(T3jr8Z#FH1Q?;OFG;R8ka zhw2MEa3>*~4RCxuu~e~&Y()0JKp5&y;^0qeCX@^#G0CiNotu3EoNXwOd2tm)YxV3j z(lIczeo`Bpv;Zo(D#q5o1l~VNkzNR&oQO631~ek_-)t!yDLFxC8zLc5 zzA5u31+Ya2zcit^Nxv}OJ*_AGdSw$c9`klgjEsz>d8(Gk>ekk|>WH)mHd3*8=+W}S z&V}WA4{SOyBldbTGH^Hz=c-m1#Z;n3{BAGhi^pH~)sU*{r46iI92MyQx55{c@`-C} zT^TB)l4KD$G1mp|3m;X->n1xGC~LJiwfC;AIXZvL{I-aP>c{V1 zsz32=_SwbP=?G;_*$}kfQu?w>C;6mTtb{}AvcWhtZTPESx6$O$RUA^tuIQ$CpebjY zj>zP9@9pN|?fd%qEFCPQ^~T(-i{wF1-!(k z`-`a-g7lhU9)mW9|81MIRw-f*z@rpc2!>=bva$6}O=Wvpj*pMOEx7kIxzNVPNB>*7 zddORr$75!Ovs!Ido+LaeDJku}eKoJ2-j4~pJVndC`d+$yRy+9y=s>6(g{q#Cv+4fe zzBY)>aizk%$z#D)G;*g7J*y6D5;B(qi%Zkirt6P`<`RTYG*I4)_jyJR>5vg*t>+}> zh?)u8FgKZwhBh=NN?H!jt+}>#A$C-pth6+qseA|R`Vn7MPgq2R>LS5Do6Vm_po$&Z zwe!Ht#3Ztimw}l%MK=3eX|hifi`juN%D}MRbHASkxgYJ*^xCdkQn#--cQ6dGBXtd!;Q{7Z#=SG+NOv)u}7RIr5kUmrY_hhczpV+bhX^-HpX+upA9rm9`+ z_gv)u?CrH@e)9SU?+lr^1P8NSV0x2V{SiC`-S!fa}4 zs!R>|7`fxmok4%BoTca*I5a$L@8KbsX`^2@AuKCKq4fB{H_$u1?(f( z{O8ZAMfyac<>uziWG;H)`;*M^DBAUp!^N9joe@QNNv^s#>Q8#`fgNli2%_RmL7|3Y~F($YK0$_>JM_V5$!&47OT*GWU= zkHrNBd_>x$f>{H74x%|&C_6j5F7J*$+Siw-yC|(lS~BjfSIvGeJD>x1*_P_fDkL~8 zPMnO##i3BB&F#&?h6X;7Ns|TnC1xLU^;oB5ydvTrI`}RL2}yQd9(}MUP>wXmrV@p6 zRIux}g;XvLUK(O>s2AyXZGansg8`WN)m7{M@VAW8(!Lk9@2ulw2Eo3>FT1T*P6pN! zOiUmHU*KgMk+j0NMDq!-Pr^tmZD$Axz8BgvN^2Mk&bBFIoQF$favsVGG9TG0$1l9Z z#l^+o!3+HHHR8(m8mL!lr8mtK5fO_fnG#`< zde!&H#d7Fl97^-AhZf4@cn=0!u zrM=~{%x9PM7wAUHcOVbEbTcco@grS@u#i?i@+r^bETDAvnxZw*xj+av*`(jOx(fY< zA|G9PMuVsJmSyYl5Be0i-4@%-i=7gK7mIIG&j2h%Et|xDl;CBK>o8Be-1_7|6hZ^QY0%k>W2r15CN)sSO|r zG<_?7tDQhyp}Y0UEYRwiP#cAKAeV+xwaJgw)vqJO2b{@0g# z$t%Qi-#lSPKl8alO$V|;w88(?i9>~lwH%(=b6%R*8zm#dj5bqcpjpdP)%FF8a@-l3 zO;)5f$e|-#&STis{l);TbgpI0LuqN|i}Nq{FBTYTntRh|qu*y3f6|S`&a6JoGbVf~ z+Zf9b#UZ!ftcsnf97yC9I_ulpz|^Vjnb&qJ-g(LdNss8{dH;vxc;5VcL#)mnp0ZK6?0&wiA0N{x)J z>Q3Y+Q!}odH?gd-6`;gzrznt(#X8vGCU#V6ZQ7-UE%~-0&*xDM;*{n^L(a#iw6E}? zuF~a@fdZYXAoHCemwPi`sMdZh(WFXx%t58}WYmYh^?9L{DP(ER4Pk{Gf=To#2fsR< zMI6wZAhfwdNTNX~iN)IGC{E#!+i0F^kWQuV(0;2tJJZnCPOgoatFCZoMq2w5nl!MM zf%FE;VtG}YUn4!Xl2?pgg8PV5%7TZfMqHbkFJwvKWT5uz_{)L_{z7+FE(;1FGg4Do zsoGaT=nqToKCMcLrax(>X%ny^19B`|0ih#J4rQk=_AF<;$V}u?Cyzs9t*%Xb&ymuj z*0A}(q5?E`-yKR$mEdQe>7Kqxb{_T|i${K8QYyY5U(WhD{9I-dJCl2*!q~qkQFjDlMf{X*VdnFNCEjvo>D@MGW zpG$Ss=W&5;|JQ5tBzxWa4lWxCeI%r;tzvoMz8cg<_UPu) z8}8TW!D5>$c{~_m|MK`)KcNEYj?(*zm0Vq}JKHYxOt%%!NR*8z zK*{-0H#Q`HiCRfT>Gkt-c1T5SMJ~MGyNPcE?vWRXheaTNbW{HZz5uhhrq@H*>1N&= zUBI&XsqIa*OP1V1A1~WMLjxxI4Yho75E)k=7KLL(0jUrWmdJbL>MNCHKF_&7QPeur zncB)0f|;vrSh=#Un7B9genJ}|ie+K#<31kDJ#z`b8*i#MTc0`1Bn_G06LwYn98V?l(+Liy>2KV`-1>fAPS}9mjZr$ahJ2L@%t8nnEBKM zm$iW)j^4ZaR_82~Jx91mbWeH|+?7_5#Q*`=w<-55-1cJWdHzzE)3J5o6N=ORC?+R8IT=}^ZmrK(L9fB^a4pJ zcQev*xVhg}(uN^UWVbn=un)5Mj%D6$De2v|L_qK^+m1RIo&l6w!0y@q#~wn&W>e90%rI>trS>cphP^w%6^PSM@YwmOM$xBW1rW#EqezS>??OK=FYUQ-aQ$p-?# z%qZrLEdd5RBD37ygtMD?tc2Ud#KO+B3F>!|T%xNReyz`gaAx)4f5q4?Te1VsWcP+b zP*58SKO)-rjp7x>_+ixg9y}apE)eJ2En(oX>a7i}Kd=NN_Ewj)L|!IR)3)s(os8!} z`z!DwkDkMBww~M&|Ct$mg32(qz~f8Ao7zpO$XGfD+_(E$2)mJ*ziaC%%k~r^5a5OS z=i26%iM!b${DBI-q$W@9ev6=PJdIgD$Z2c;hMkT)v_(?;d)+IH;tD2+R=izd&(K7- z5Q$?k)C9vM{06qg;E@jJ3ZUMTEnh-0 z7+|Zsh(hDF!s6B1*hzFkOBBUKMgN)?v&qAoD@ii!`#As2B>F{~lGmm_qaF|1 z{eZWQuZv#pkYnIQ+jI=AA%eI}4$kf{zaQAjF-7j`T*F3z35IVdxT=KI(5NOga;s<; z%DZ|KRDc4sz~_GqP|>%M(uE4N9QCaSvig-OR?=`Q}IiswKD>u5B~)1baHyi zj#ZbCkc(qNQvNH zx}BztxAiVn*Kf4ct3NB(mGmtLR#~wl4^|9*vhL-)^?sR9AC|j3V)pZ%gINcKK7L)X zQ5lgg`)`M?>ltrBhM@kns|554xhdy{y%3!#uq|HS;oTvg1`at)BwoUi2aropUIixq z@VWsxn@h0wpr(F6qHk&&P3$t+3&YmWKJN9GBQ?8s!NIMezj^#Lu=eE<=h&wrRIr<^ zCDN}Tw_K@!QrVUF0gz1{sBrIMr>_NpaP;zz6n&ZBCzcW{yId`>><2bYvQl`^sz@%g zssV3B)S>jkm6SW{S`G+KY1~qw)Ulgfc4m1h!8KS*f4)Dbf`9$4|NSgeqhvhiM8_RW z{-?Ezl9I%v0frrj4`sj?#hAqtFIt@vvMYupLsP>QdDYp8@4%vQzgo@WVM((=Dn|Y8 zPPiC;+X0{@_JqKZA~oh21-BMohLT(!MxdnBg|vc7dKiF0cFsiHM9M~~dkYVy<4N1F zfj5NzGPkP5gSxt7MZ&}1^kJ*ZV;h<5Qj@vtv*1)v?LyQ`#-ldulga}Hm=2t;nb_Dc z0^`o&oD^msi_gimG}F0gh+l?*v{3OfjbldstomY;9_H3&8D9?_F@3m$_192W9tTV~ zD?{uyjpI^^ME$FC_-2Q!H#m)rCs63AU=Dn|1oarOd!_DpkZz8JFlPP(W#g2hF#KQ1 zNliHdPdYCrAMSKV008Op>%FnrH5S-N&{Gr04^-f2KHWc)3Hy>)7r}w7*ZIren8;&MRmC4z?$sRwM5WBBG{JH-E?^~&h!!l@TEEhBB4M+Rh@7b=h`%(<&BQ|=Y0^C&70UFZF79n%8TxV7h54L7t zl(|;9k)5bxhW~NjuufZXv-z$5_t>qiEt9S{P*6aRZlk3(3jJ)|+@5aQCG+G4a@NV~ zfOEwqC1D|opu8%A9xk{@RS#ANh~T1#;1`XN9o@@}s-L>@gtW;I!H zYFRpej0cZ|!A#7;!IX~p24FGPH%D15R-7v1;s8LnJeAZ;kF~4r>)%^_`MiT!_^A)Y zT#}l6puEn_YR~#x&@mZLEAZ=Zg}tPGMIm~6ZH3OBmA$(^NuGe&W>{xAm7cVpUZ^8$ z^t{tuIO=TU;X&@}>zn4t_~9u&0OK%w^X|sS6DYqwE)wAQR>~Ck4VpKc;Q!!aYKj45 z>^F!qB&q_FJ@f*AeGaDnEXm{IWX+YzT7>uN;J+j(8uB-GunubX2q5f5rUvJG75 z4yLsCW@;3_1TzVM>X1O+llKZ2-RgX*dUn=SvdNKn;yj&fk9}}~ZK3QkFa~^apppmo zAwgGuFP(W90R60U@6*sxvIGTKPde8>*e2}F&?WgP)65Wu7C!V&&tp@nso+@w$?!8Q za=EKq8^}PU@)XQFXLyxH!49MZ%F;PA^u~zh{X7VUCnVb|G?eA}^XFYCZS?k1XUscX z=zB()zwJSlO}~-V_PutpRnP$-?VVC5O{8Dz#daB0ol7IYqMmAN_s|7`hDZ6vmt{n5 zd7@j}uF=)FZYX7HtLLNKD}UOOVQA-TlOY0g&kPBbDP`49tT^gv3e;a06u)hp5f?!tnqxQo$>h$Y_Yd!!E>Ra;(`;-kG z5N$sx5?bCW$+HUJpY7%TtOFnq;?hz(fB%Q+K|$WP%mvF`X|&aII04IN%c;wA=lo|K z^71Kp?XcL43%{Ho?Ox3lpozlRMpKISOPA=@4`(rza&m-ohqg9auuBxTD$upnRW49e zt#527e^{;H)%-?17Xu(FN&g)N1H;`5O{JDsQe>)n`MjFQPQK*xXWtEod+ezGr`?7l zrWi4`dVD^KM*xcRg7TEi;f-p~#dZW{m#b#RhQULu{N<9xjVEHQ<1LTP?v1ZPuyx(O z)8~)5xu?4sQa-DR8$uYD`k7`I-4h(u%rzs5VfJf7k2YfcuW+4zK>eSOeb9lLX9~eP z)x9#-n9Gj|1y-o`48Sv({Ko|tb*^i7`dDlm6f`a3z?U4|uTjn6`A%cD2HHFJkg?ytyVyh~-vOPTAG9!ry7V7c+>fCh(1f+Hk)@?!8Z?fwQQ;3^ zo&hN4Qx$~yYyFWsTC%Rho%uJq%fHohvE|wmp-CfAIj5~+Yu~4tE}1~gEOcU}o9|DB zMzWJFQjUTkp!snmI zUqF*PkB(|KP&+-Gg}k8mqX|Z^wnwx6bAHS6y@pG7PpMT0_wR%+eEpDEh)B!ey4$8$ zc1yxMV4b{Qh ziUs%fkiuHl<1=%$o`pwx^xNoWPhBIytd#E(Ds>A$GW|aVhSa_Q2z=pv#>~p%UVxkH zsq^D{)Wdja^6w;S9 zHXR-wGMFi=plFqi#@uE*F1WttLWhG%Z!dliUBH93wtfc>t&FqL1&*uy{k2x(g|HK% zqf$-kj?<`Jp@`HPF846$O14)JP4Q!fEm6W=Pu?fRX$&V1+~P-ETfg$ypHN+zxZRwq zljwvEZ7PmQVrm^q1tx02N zwy`Fxkl~of5w)I6d0`Z7Htr2wdkR842O(8*TNeAp?ufnN(~}~0?2pSX9v;5~I`!T| z2P8w!yna-0a@zH27N5vtSKg2Y9v3ZO>l;G`tcJHw*T#o#=eDcm7v%e`pxYmdUw$*- z`({+u4j-&ecVqzy=fT^%y6uhcHErX`vt*Qu$snB>HZXbx9~xx)m+E*swOy99z)+P>B% zknhO+hM>B~ouAu`x!7M{5`t6(EKaEa!OfKTtqL$&dZZh$FszWhtCbfcW;gqYS^iOs zL(_!sWej4oq>(%`MyUWxBR$mko^>kdet`_gEWT4G(~*W44}fl`7F44OfXDI>J~u&X z7H}itbxz~8MKE-mgH2EAfoQ%4k1OF*XRXRflYBI@oe)=LW5b?1x5 znf6DO0iyqTW$B40Xo>V*?W1xKMfu*C)tmJU4nCc22j21BJ1Wqv1fLF~jD0n8C3JVF zl5G3TsutRb#ea*_leM+~A=TwCF>f01dBBdHrGaX2DjszP z9HjbqasZqp|1VBetD^Y-!J;Z7LB>o_EjRYYJ;Fl7lZoVI0ADB!?D?~8Vq8gf%p`uN zuSg=`1i*kM5?2XxQ`Y@$Z|_97L>|E)k=3>_j^7DqQKZ&@>taJ{it|f?rU{o1eq^=G zH@LJ$llW1~g|V`#WpY9$|4Y~o`X6g(`3`wZ*8>?Me7+LTf=#)qYtGiL_v5aoVRvVT zKDS?9{RWNlKc1wf*IW$ctx6fzOD?mgj6S$OHr&q9d_i#!7V{kiP+qLPVWbwIqi(}c zbCJKIwMSj9qHKg}YLlC7FrJ^^lv2P8TbNsJ-fc)tO#H^iP-C~sT#4J2=J~BgCP3m+Qyf&L&|26GI@^z zBpadt^~i(NRPT7J!e=|EMvk7y!o%^pVYgS(y@hHws%GDJwgLYuyW-%Dg~KN>7F2Om z)SnYsxt>mAlH$wQ3kGkRXEUNbAnM{4)9`E7>r=?iD3D1f!mp-qRhXd6ef*FTgRb6j zyu!{yg>&&cQt=nfBi=dJ7NjPCjR#yH?=;Rk_t^)p9^(my1eN4Fu)2DfhkVY7 z-z(n`n{N$z9=Ml28w!OZV>c{_2K#-acx0lPR%ofH&^SEpYD3`?9MRh0aJl_#m`9_~ zJiCvCE!>zPS+Ig~)kXPT5liE196*-?wfxho=-18&pScanKhd$@t-^7yGW8E;dG1tN z3`jhhJiT!-QN{-Ep}gP-jQdxWbq19ylM6a+l{a=eUej z`3{TZ`gC4IM%eEE_=(KQlUUwYMuf3~aQhhia&~#*aM>sA#yHJ8zR>Z|&Hy%GM4*wx zWT3PB%T<0(Ieif>S)R17_^UCFtLorKfdZhQ-T6lhA~eaZ!h1k{rz5v`lHVih*G3>v z5yjp#2~w5r_d!LrvD$|ux_8$YzXD5D2k4$H%r@qrbbWq-SvWCY5l|-pP)&ayO?{nYtKJPj2y@Vu5-S{Jb>W5EBoYbo#EBCnf*=rg@;-N* zh}Y|-PoF+r&hz};MI;uBk;jf5tKvBBjv77}1t$^=2IUtoUNnoM_@xq|vR#NqqhxPy zuh!@D(GG_rR$E&e2Y^Tv2WRk3}ua8~1azzgS=g*%ns;#Y! z&75<3tkda?YBU;Qx;+pG$h*6{4XIR0=5#tEcDp?}eN7ZaT)uo+?{>SD6h%p!Hf@?P z7!3Sedoz0Ewbx!-_R1@-*h8TZ#q&Jz^2;x;?C9ty27s=vF8%4#r^{yc@9gX}wzs!i z0f1#$82}`cNiv;If9{o+UV3T8n{U2Z)z#HyeD1mDoC5;`N&x8V>r=h(!V7gnLqjTx zqNG<}eYN_`nKLE;xOwxY>ctmdbbS2r$9lb9&)v9jL-+ji&##WfVq{((xq9`g@tJ3y zxmj0N7Y6{Erp0sT&RMo^-yX=+e`R z05mo>h7KJ%)Ntj>75&DI8z*ymWG-wrTf*n_DZ}A#)}@#W#l^+k^#6*AibNm~pgE2s zdV6~fD_5?JPG769ua89{5rxa;QqP@3tJMkygCRZB(~BZfR8+(R!0_-evospBK@d}R z#N%<2Wm$6OnpY?kB1KW+~fcWN>g$Ns^>kQ&W=wfOI-t5VbQH417gJMSNgj zKsDXwcDort5M&Jv4UxsnmDM1Z%cV^u5@a|WraC)2P1V)aQJSVjr_&h?g+hw1t}Z<= zr8r|_W6J4qMMXs%MNy*1<(s6h(ME9;MZ4WtU{ZOk4GpQbZ{I$)Zr!>?Da@Tf#u&L9vE-$d4S`6K z>b~XGjD_R!iD*(DiY6&aMo0!V!yB~>Z#AgXTh=U#P&s06u_zdkes75W@$rkcUiY{b zk_1Q+fX{wE|1ScP1dBnH`sNm=?~w=VCkjQVAWKUWfro#3s@@ZbF{^DR#FHru_(I4= zWV(IV`j!44d~3__@67@QI#>;ig=o)2oLOVH;O7Tg@b00nqgb!XIqp{E5>rNAOG_5a#2E#;LK+D zBJ-aAqy<7MZz0H=$jM8CMwSx_qWLqSQPKj$47r34%Q1e!lM@*jLR^d}~X?NOMC; z#H1LfAo0XJvJwFBK;pI%;eRn>EI{jQ#mR3!pWP6S;wS3lktoNIxQpTe{o>5 ztAhJ?3p~HIK@x~*56sRlGo?9?>`xVO`L6!FW2D5ON@csY|7-UQSES#uBJx`cP{7v- zOj+Iqjl7AJWg$Wkkajc_M-lm*Ip4YR22)jdBFzou!A$LDy)v!B5S=ScE(ylwyel(# z6Zwk68go_|=T%Ecf{>;HyTDM@MN#xXGn?A79>7CUW`$)UsAeAHbx!g|gil<2!w}n8L<$f4BQ1BCCM)=>U%$P?c z2~gG~3qj<||AeDx8({uZfGF%nx-cFg^L!~omjhLYrv9frKWR^x?)41r%-5p1_Wj8hqj5DX7v%Q&oi$3W=|mXq$>YlW0gf%G5^c7y?;5J8ux zz*MCmH|Ji0OTbWpa+TAT)eK~F$&?9~5SgVC7$ zpa1ro{UiRM;^(K%SuF-#dhd?)0i$jzKeheOMbn3$^y#WfOl&Y3&scHE=#g*z@D0bN zwYAZ|-~X^X9B0X+@3xl$!1rJM&#HmZapu{*+r96czF>LnKR&SNwMzc&Uv-v>qKE;{ zIDPy~cge5Me_R9r|9#<_vDBpF{_gdYmG*KA`|X`y@gF?ewn7jD;?~d@bKuB3)jJ<> z246b3cR(hS;n|&pufbV=l7E%j!AM-b-nk;Cph)TdRm)=~Mm^_pduJ6gT!UVQQBdOg)%F+w+*h}}kEBS+7YNZ0 zuB{2@vNLC}U%x$~TxBndnT6cdxF2addnb_$s~rB zOHHd+gpd4Nd*zn<>cSc&BkDCuE|ubB4qGYfu$8iUt!iE(bl6H+|728AWi_U2mzA=$ z%Su^VPD$$=Rnf~XwNz;}0V^6vl1+ci6yoD=7- z_301)<(?q`C^H*4vq76)ZgE_1@axcrdEi z+p@tw_kN|tdhRd2v1#=2AHMF`d~a=Vcx;mX-Vb){@kMM6wO~|A>q+qEXU!zQ9mmWb_Ef^MstD3i^q~okm8F+-)R) dKcc`unPw&?X(v;o(_+&^9j6^TNhdm!bdrCJ#A%1=M8%mVjhQAz zMQEpt+<>-36!8aE0rdjN9X}3^-*9-^_j&i-^S$4D+}_^O=9$^qTlQ{upAX;Xd7t;) zcNgF*^J)>{zJPyy_?nKa@;vNnUA^s~_5TJA0Z~L&xO|H&Sze&HK_ZKhESJb)Aixa& zGjIryQnK1a7F_FG_s1lpKult)+wW$^EHE=Zhd{ExsHqc1%^h^x1yng%4um2EBSHa1qm6y*u5HSp4E@R? zNXh&r{T!^Jnufl-45q6FURfc56d@xRRLWbTQj+vJur$Afsw7l~YU;jNOV%Acqa7#soyp&%Tpl&n=hR$X;#Z2FukiCHB9 zv?b(lcrd7zujM?g+VX#(;|r>yxdMs`5X@dyzo;IJP*-KZg2WWk^k2cvysaCbjyQDU zoI!Tw@Hu8afbpRq$vG%eA{gugy^o)lk``$`fkDIT#p`DFKJ!N&IAU{+Rc=Op$-EqX z!0X2AXGQ7}S>UidlZVapBP5*Wvpw>l+{j~o%{}L^>i0H&&fW03qE&AwtLe6y55&!E zUsZIZgm9vAoG)M|to>lTcpBR3kAc;6+igGk!NLptg;#{1fvI}@T(EhsDymIlTStou4{wTZTp;HX+fYdeA>C5ShQb zq=r?gkcb~wB^Xo+D#jIzTj#UO3>ja~hvdbchhsVaN+pFAs~)qEESJ?)lV8q#evsxE zwDrH-58r)hkK6a`_InhJR;P!T-#F;?c-+u!mZz%OQ&<&=V>D{ZFK0;@@y&BxQ%P+3YcCXn2c1;4j@az4MyK1eZ zW22u84*Gq-ADlcjDF^clhEZ(z| z!1UBKOc59p6BBfea2y@Pm~>ch?-oQy-gip22xW2hO4w5N9xQwODOs)s{bd94{*|MN zi46%1gfLDPTw?Vsb((CIG)*Y-#<0~2gBFWlbx2CgsViXbyYB^@XjKIV z!=|slpDIPRX1u)}^1i(~EMTMzc1eQLedi8bztJu$7_{bd&G7xg?}e?L2ue8^KD{Qh zQ!68vUAcS(($mu*Gcz-6aGaD=V;L0sG0qc&yxPdK z(~OO-#feA$Y-xHrL`6l(ez9j7&p=Mja`?uI6^g(S{T2Q92=%lKkLa)Hzqf>JG}&eX3azB1M7qdf*md~qub;v0-Mb(zE>6*qVtyNa z!nrs$CeSo@NIrpF;dQF|6l~h~tYYEB`^o(F4W~tfW9pQA0y)B~y1E+lOO`-M$uo+! zNTQ!De&JL!85^;ECWbRMr6%TJ!>hTu8B%n~P*(PHMZutd=qGI*(Qo3v&mwTlez zIxN&N?1!NYFRW)~Nh_f=7~Vhh7yXt}As=rL#KTV;3rTn(m@!f-g#!%lAM^LP6gY;O z!9f@&#T{nyKyptY;Z^s~I!H;@g$)e)h5n(R=r0J$$O5ulsdO92a9PkCWh8I@8moHs{T|0M#wKK{4gZ_;ReZxxWaD*nU3aMh{ zh=b2PL4?<}wl>g4N5i(QuP9dedNcF`{XxH&YN}kDa4F+C_k;e>vmpygg)7~#o`B9qOQl@O(Yl?e&7E_i6g8ihil%*%#&bLvo$+1X-31+!lk^1FYCZ_4a#n-! z*NuLlKfHf|D@Ra|m8b`&#ua@*inSjuS_Bc(<1jta4~v!+%o{iz^+#z)VHo|Y{e~D$ z`-p0)<_<>yWFUY_$pM!%jgG*&lvJ>f4$!2A_n89;v2ZBn*4a`dG*G1_)@q< z)Jnj-=308fN76Vx1HVkqHtieT{ifjx&MsX>u+rt>{!q8==IN{41= zK~xZi82(pKLE5+^)qufn0fLz#;qL+lZ97rT-{pfN0BR+GM#!gUL_jO!6JX)}yA)8Q zCK?hKXN4UMb|!DseF%1zeD~b!&AAT4%q%LxN z1tH;NXekfZ&qXO1LLwA2yzg|Nt4nOEA2cQvO_g&wM{ti$m#uO<9W4Gy9}@#x)6$$E zi8E6XRxqFt2*SXjP5xC52<1?3FPLU#_^(?T80DW1<^W{@_gpy->G{PE(ZVr-8{5HY zW^t(*u#mp8A%(uS0WFR|!+qEUPqN!3_?iZUFhUBrNlTAY*?{UeH8=>L_V>9$5eKfe zcAKr#BMthB>J)SqJ%~pQYEv%)KQdJ!!m!(lBgw%_+QEu<}*M8wp$hd3?c# ztD^K;EdiB874@i(A&hp?1fxPOj*Sv9HctxSq0o0(5HRX{mqnnEWU`Pz2`wD=r0we8}@g0u@Ln5&aO~|%2y_n{a1hi00-6?7X^q( Q0RR9107*qoM6N<$g8M56QUCw| literal 0 HcmV?d00001 From 708f1b009b9b23971d73bfc7bc09163969ab6e00 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Fri, 10 Mar 2017 21:36:33 +0100 Subject: [PATCH 16/17] Add integration tests for MEDIA_ALLOW_REDIRECTS --- tests/test_pipeline_crawl.py | 163 +++++++++++++++++++++++++++++++++++ 1 file changed, 163 insertions(+) create mode 100644 tests/test_pipeline_crawl.py diff --git a/tests/test_pipeline_crawl.py b/tests/test_pipeline_crawl.py new file mode 100644 index 000000000..1f5b80954 --- /dev/null +++ b/tests/test_pipeline_crawl.py @@ -0,0 +1,163 @@ +# -*- coding: utf-8 -*- +import os +import shutil + +from testfixtures import LogCapture +from twisted.internet import defer +from twisted.trial.unittest import TestCase +from w3lib.url import add_or_replace_parameter + +from scrapy.crawler import CrawlerRunner +from scrapy import signals +from tests.mockserver import MockServer +from tests.spiders import SimpleSpider + + +class MediaDownloadSpider(SimpleSpider): + name = 'mediadownload' + + def _process_url(self, url): + return url + + def parse(self, response): + self.logger.info(response.headers) + self.logger.info(response.text) + item = { + 'images': [], + 'image_urls': [ + self._process_url(response.urljoin(href)) + for href in response.xpath(''' + //table[thead/tr/th="Filename"] + /tbody//a/@href + ''').extract()], + } + yield item + + +class BrokenLinksMediaDownloadSpider(MediaDownloadSpider): + name = 'brokenmedia' + + def _process_url(self, url): + return url + '.foo' + + +class RedirectedMediaDownloadSpider(MediaDownloadSpider): + name = 'redirectedmedia' + + def _process_url(self, url): + return add_or_replace_parameter( + 'http://localhost:8998/redirect-to', + 'goto', url) + + +class MediaDownloadCrawlTestCase(TestCase): + + def setUp(self): + self.mockserver = MockServer() + self.mockserver.__enter__() + + # prepare a directory for storing files + self.tmpmediastore = self.mktemp() + os.mkdir(self.tmpmediastore) + self.settings = { + 'ITEM_PIPELINES': {'scrapy.pipelines.images.ImagesPipeline': 1}, + 'IMAGES_STORE': self.tmpmediastore, + } + self.runner = CrawlerRunner(self.settings) + self.items = [] + # these are the checksums for images in test_site/files/images + # - scrapy.png + # - python-powered-h-50x65.png + # - python-logo-master-v3-TM-flattened.png + self.expected_checksums = set([ + 'a7020c30837f971084834e603625af58', + 'acac52d42b63cf2c3b05832641f3a53c', + '195672ac5888feb400fbf7b352553afe']) + + def tearDown(self): + shutil.rmtree(self.tmpmediastore) + self.items = [] + self.mockserver.__exit__(None, None, None) + + def _on_item_scraped(self, item): + self.items.append(item) + + def _create_crawler(self, spider_class): + crawler = self.runner.create_crawler(spider_class) + crawler.signals.connect(self._on_item_scraped, signals.item_scraped) + return crawler + + def _assert_files_downloaded(self, items, logs): + self.assertEqual(len(items), 1) + self.assertIn('images', items[0]) + + # check that logs show the expected number of successful file downloads + file_dl_success = 'File (downloaded): Downloaded file from' + self.assertEqual(logs.count(file_dl_success), 3) + + # check that the images checksums are what we know they should be + checksums = set( + i['checksum'] + for item in items + for i in item['images']) + self.assertEqual(checksums, self.expected_checksums) + + # check that the image files where actually written to the media store + for item in items: + for i in item['images']: + self.assertTrue( + os.path.exists( + os.path.join(self.tmpmediastore, i['path']))) + + def _assert_files_download_failure(self, crawler, items, code, logs): + + # check that the item does NOT have the "images" field populated + self.assertEqual(len(items), 1) + self.assertIn('images', items[0]) + self.assertFalse(items[0]['images']) + + # check that there was 1 successful fetch and 3 other responses with non-200 code + self.assertEqual(crawler.stats.get_value('downloader/request_method_count/GET'), 4) + self.assertEqual(crawler.stats.get_value('downloader/response_count'), 4) + self.assertEqual(crawler.stats.get_value('downloader/response_status_count/200'), 1) + self.assertEqual(crawler.stats.get_value('downloader/response_status_count/%d' % code), 3) + + # check that logs do show the failure on the file downloads + file_dl_failure = 'File (code: %d): Error downloading file from' % code + self.assertEqual(logs.count(file_dl_failure), 3) + + # check that no files were written to the media store + self.assertEqual(os.listdir(self.tmpmediastore), []) + + @defer.inlineCallbacks + def test_download_media(self): + crawler = self._create_crawler(MediaDownloadSpider) + with LogCapture() as log: + yield crawler.crawl("http://localhost:8998/files/images/") + self._assert_files_downloaded(self.items, str(log)) + + @defer.inlineCallbacks + def test_download_media_wrong_urls(self): + crawler = self._create_crawler(BrokenLinksMediaDownloadSpider) + with LogCapture() as log: + yield crawler.crawl("http://localhost:8998/files/images/") + self._assert_files_download_failure(crawler, self.items, 404, str(log)) + + @defer.inlineCallbacks + def test_download_media_redirected_default_failure(self): + crawler = self._create_crawler(RedirectedMediaDownloadSpider) + with LogCapture() as log: + yield crawler.crawl("http://localhost:8998/files/images/") + self._assert_files_download_failure(crawler, self.items, 302, str(log)) + + @defer.inlineCallbacks + def test_download_media_redirected_allowed(self): + settings = dict(self.settings) + settings.update({'MEDIA_ALLOW_REDIRECTS': True}) + self.runner = CrawlerRunner(settings) + + crawler = self._create_crawler(RedirectedMediaDownloadSpider) + with LogCapture() as log: + yield crawler.crawl("http://localhost:8998/files/images/") + self._assert_files_downloaded(self.items, str(log)) + self.assertEqual(crawler.stats.get_value('downloader/response_status_count/302'), 3) From 871134ee22653f7f074fbfb8f3c393ba3555a6d4 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Sun, 12 Mar 2017 17:30:24 +0100 Subject: [PATCH 17/17] Refactor to also test FilesPipeline --- tests/test_pipeline_crawl.py | 79 ++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 30 deletions(-) diff --git a/tests/test_pipeline_crawl.py b/tests/test_pipeline_crawl.py index 1f5b80954..9b81f827d 100644 --- a/tests/test_pipeline_crawl.py +++ b/tests/test_pipeline_crawl.py @@ -23,8 +23,8 @@ class MediaDownloadSpider(SimpleSpider): self.logger.info(response.headers) self.logger.info(response.text) item = { - 'images': [], - 'image_urls': [ + self.media_key: [], + self.media_urls_key: [ self._process_url(response.urljoin(href)) for href in response.xpath(''' //table[thead/tr/th="Filename"] @@ -50,7 +50,15 @@ class RedirectedMediaDownloadSpider(MediaDownloadSpider): 'goto', url) -class MediaDownloadCrawlTestCase(TestCase): +class FileDownloadCrawlTestCase(TestCase): + pipeline_class = 'scrapy.pipelines.files.FilesPipeline' + store_setting_key = 'FILES_STORE' + media_key = 'files' + media_urls_key = 'file_urls' + expected_checksums = set([ + '5547178b89448faf0015a13f904c936e', + 'c2281c83670e31d8aaab7cb642b824db', + 'ed3f6538dc15d4d9179dae57319edc5f']) def setUp(self): self.mockserver = MockServer() @@ -60,19 +68,11 @@ class MediaDownloadCrawlTestCase(TestCase): self.tmpmediastore = self.mktemp() os.mkdir(self.tmpmediastore) self.settings = { - 'ITEM_PIPELINES': {'scrapy.pipelines.images.ImagesPipeline': 1}, - 'IMAGES_STORE': self.tmpmediastore, + 'ITEM_PIPELINES': {self.pipeline_class: 1}, + self.store_setting_key: self.tmpmediastore, } self.runner = CrawlerRunner(self.settings) self.items = [] - # these are the checksums for images in test_site/files/images - # - scrapy.png - # - python-powered-h-50x65.png - # - python-logo-master-v3-TM-flattened.png - self.expected_checksums = set([ - 'a7020c30837f971084834e603625af58', - 'acac52d42b63cf2c3b05832641f3a53c', - '195672ac5888feb400fbf7b352553afe']) def tearDown(self): shutil.rmtree(self.tmpmediastore) @@ -82,39 +82,40 @@ class MediaDownloadCrawlTestCase(TestCase): def _on_item_scraped(self, item): self.items.append(item) - def _create_crawler(self, spider_class): - crawler = self.runner.create_crawler(spider_class) + def _create_crawler(self, spider_class, **kwargs): + crawler = self.runner.create_crawler(spider_class, **kwargs) crawler.signals.connect(self._on_item_scraped, signals.item_scraped) return crawler def _assert_files_downloaded(self, items, logs): self.assertEqual(len(items), 1) - self.assertIn('images', items[0]) + self.assertIn(self.media_key, items[0]) # check that logs show the expected number of successful file downloads file_dl_success = 'File (downloaded): Downloaded file from' self.assertEqual(logs.count(file_dl_success), 3) - # check that the images checksums are what we know they should be - checksums = set( - i['checksum'] - for item in items - for i in item['images']) - self.assertEqual(checksums, self.expected_checksums) + # check that the images/files checksums are what we know they should be + if self.expected_checksums is not None: + checksums = set( + i['checksum'] + for item in items + for i in item[self.media_key]) + self.assertEqual(checksums, self.expected_checksums) # check that the image files where actually written to the media store for item in items: - for i in item['images']: + for i in item[self.media_key]: self.assertTrue( os.path.exists( os.path.join(self.tmpmediastore, i['path']))) def _assert_files_download_failure(self, crawler, items, code, logs): - # check that the item does NOT have the "images" field populated + # check that the item does NOT have the "images/files" field populated self.assertEqual(len(items), 1) - self.assertIn('images', items[0]) - self.assertFalse(items[0]['images']) + self.assertIn(self.media_key, items[0]) + self.assertFalse(items[0][self.media_key]) # check that there was 1 successful fetch and 3 other responses with non-200 code self.assertEqual(crawler.stats.get_value('downloader/request_method_count/GET'), 4) @@ -133,21 +134,27 @@ class MediaDownloadCrawlTestCase(TestCase): def test_download_media(self): crawler = self._create_crawler(MediaDownloadSpider) with LogCapture() as log: - yield crawler.crawl("http://localhost:8998/files/images/") + yield crawler.crawl("http://localhost:8998/files/images/", + media_key=self.media_key, + media_urls_key=self.media_urls_key) self._assert_files_downloaded(self.items, str(log)) @defer.inlineCallbacks def test_download_media_wrong_urls(self): crawler = self._create_crawler(BrokenLinksMediaDownloadSpider) with LogCapture() as log: - yield crawler.crawl("http://localhost:8998/files/images/") + yield crawler.crawl("http://localhost:8998/files/images/", + media_key=self.media_key, + media_urls_key=self.media_urls_key) self._assert_files_download_failure(crawler, self.items, 404, str(log)) @defer.inlineCallbacks def test_download_media_redirected_default_failure(self): crawler = self._create_crawler(RedirectedMediaDownloadSpider) with LogCapture() as log: - yield crawler.crawl("http://localhost:8998/files/images/") + yield crawler.crawl("http://localhost:8998/files/images/", + media_key=self.media_key, + media_urls_key=self.media_urls_key) self._assert_files_download_failure(crawler, self.items, 302, str(log)) @defer.inlineCallbacks @@ -158,6 +165,18 @@ class MediaDownloadCrawlTestCase(TestCase): crawler = self._create_crawler(RedirectedMediaDownloadSpider) with LogCapture() as log: - yield crawler.crawl("http://localhost:8998/files/images/") + yield crawler.crawl("http://localhost:8998/files/images/", + media_key=self.media_key, + media_urls_key=self.media_urls_key) self._assert_files_downloaded(self.items, str(log)) self.assertEqual(crawler.stats.get_value('downloader/response_status_count/302'), 3) + + +class ImageDownloadCrawlTestCase(FileDownloadCrawlTestCase): + pipeline_class = 'scrapy.pipelines.images.ImagesPipeline' + store_setting_key = 'IMAGES_STORE' + media_key = 'images' + media_urls_key = 'image_urls' + + # somehow checksums for images are different for Python 3.3 + expected_checksums = None