diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 4f6fa21a0..97daa514e 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -33,7 +33,7 @@ from scrapy.pipelines.files import ( GCSFilesStore, S3FilesStore, ) -from scrapy.pipelines.media import MediaPipeline, _MediaRequestFiltered +from scrapy.pipelines.media import _MediaRequestFiltered from scrapy.settings import Settings from scrapy.utils.asyncio import call_later from scrapy.utils.defer import maybe_deferred_to_future @@ -43,11 +43,7 @@ from tests.mockserver.ftp import MockFTPServer from tests.utils.decorators import coroutine_test, inline_callbacks_test from .utils.cloud import mock_google_cloud_storage -from .utils.media_pipelines import mocked_download_func - -# required by persist_file() and stat_file(), but as some stores don't use the argument -# we can pass this singleton to keep type hints correct -DUMMY_SPIDER_INFO = MediaPipeline.SpiderInfo(DefaultSpider()) +from .utils.media_pipelines import DUMMY_SPIDER_INFO, mocked_download_func def get_ftp_content_and_delete( @@ -94,16 +90,19 @@ class DeferredFSFilesStore(FSFilesStore): class TestFilesPipeline: def setup_method(self): self.tempdir = mkdtemp() - settings_dict = {"FILES_STORE": self.tempdir} - crawler = get_crawler(DefaultSpider, settings_dict=settings_dict) - crawler.spider = crawler._create_spider() - crawler.engine = MagicMock(download_async=mocked_download_func) - self.pipeline = FilesPipeline.from_crawler(crawler) - self.pipeline.open_spider() + self.pipeline = self._create_pipeline(FilesPipeline) def teardown_method(self): rmtree(self.tempdir) + def _create_pipeline(self, pipeline_cls: type[FilesPipeline]) -> FilesPipeline: + crawler = get_crawler(DefaultSpider, {"FILES_STORE": self.tempdir}) + crawler.spider = crawler._create_spider() + crawler.engine = MagicMock(download_async=mocked_download_func) + pipeline = pipeline_cls.from_crawler(crawler) + pipeline.open_spider() + return pipeline + def test_file_path_query_parameters(self): file_path = self.pipeline.file_path @@ -254,6 +253,107 @@ class TestFilesPipeline: assert result["files"][0]["checksum"] != "abc" assert result["files"][0]["status"] == "cached" + @coroutine_test + async def test_file_stat_without_last_modified(self) -> None: + """A stat result without a last modification time forces a download.""" + item_url = "http://example.com/file4.pdf" + item = _create_item_with_files(item_url) + with ( + mock.patch.object(FilesPipeline, "inc_stats", return_value=True), + mock.patch.object( + FSFilesStore, "stat_file", return_value={"checksum": "abc"} + ), + mock.patch.object( + FilesPipeline, + "get_media_requests", + return_value=[_prepare_request_object(item_url)], + ), + ): + result = await self.pipeline.process_item(item) + assert result["files"][0]["checksum"] != "abc" + assert result["files"][0]["status"] == "downloaded" + + @coroutine_test + async def test_file_empty_content(self, caplog: pytest.LogCaptureFixture) -> None: + item_url = "http://example.com/empty.pdf" + item = _create_item_with_files(item_url) + request = Request( + item_url, meta={"response": Response(item_url, status=200, body=b"")} + ) + with ( + caplog.at_level(logging.WARNING), + mock.patch.object( + FilesPipeline, "get_media_requests", return_value=[request] + ), + ): + result = await self.pipeline.process_item(item) + assert result["files"] == [] + assert "File (empty-content): Empty file from" in caplog.text + + @coroutine_test + async def test_file_downloaded_file_exception( + self, caplog: pytest.LogCaptureFixture + ) -> None: + """A FileException from file_downloaded() is logged as a warning and + kept as is.""" + + class FailingFilesPipeline(FilesPipeline): + def file_downloaded(self, response, request, info, *, item=None): + raise FileException("boom") + + item_url = "http://example.com/file5.pdf" + item = _create_item_with_files(item_url) + pipeline = self._create_pipeline(FailingFilesPipeline) + with ( + caplog.at_level(logging.WARNING), + mock.patch.object( + FilesPipeline, + "get_media_requests", + return_value=[_prepare_request_object(item_url)], + ), + ): + result = await pipeline.process_item(item) + assert result["files"] == [] + records = [ + r for r in caplog.records if "Error processing file" in r.getMessage() + ] + assert len(records) == 1 + assert records[0].levelname == "WARNING" + assert "boom" in records[0].getMessage() + + @coroutine_test + async def test_file_downloaded_unknown_error( + self, caplog: pytest.LogCaptureFixture + ) -> None: + """Any other exception from file_downloaded() is logged as an error and + reported as a FileException.""" + + class FailingFilesPipeline(FilesPipeline): + def file_downloaded(self, response, request, info, *, item=None): + raise RuntimeError("boom") + + item_url = "http://example.com/file6.pdf" + item = _create_item_with_files(item_url) + pipeline = self._create_pipeline(FailingFilesPipeline) + with ( + caplog.at_level(logging.WARNING), + mock.patch.object( + FilesPipeline, + "get_media_requests", + return_value=[_prepare_request_object(item_url)], + ), + ): + result = await pipeline.process_item(item) + assert result["files"] == [] + records = [ + r for r in caplog.records if "Error processing file" in r.getMessage() + ] + assert len(records) == 1 + assert records[0].levelname == "ERROR" + exc_info = records[0].exc_info + assert exc_info is not None + assert exc_info[0] is RuntimeError + @coroutine_test async def test_async_store(self) -> None: """Test that async persist_file() works and is awaited.""" @@ -648,9 +748,24 @@ class TestFilesPipelineCustomSettings: request = Request("http://example.com/image01.jpg") assert pipeline.file_path(request) == Path("subdir/image01.jpg") - def test_files_store_constructor_with_pathlike_object(self, tmp_path): - fs_store = FSFilesStore(tmp_path) - assert fs_store.basedir == str(tmp_path) + +class TestFSFilesStore: + def test_constructor_with_pathlike_object(self, tmp_path: Path) -> None: + assert FSFilesStore(tmp_path).basedir == str(tmp_path) + + def test_constructor_with_uri(self, tmp_path: Path) -> None: + assert FSFilesStore(f"file://{tmp_path}").basedir == str(tmp_path) + + def test_stat_file(self, tmp_path: Path) -> None: + store = FSFilesStore(tmp_path) + store.persist_file("full/filename", BytesIO(b"data"), DUMMY_SPIDER_INFO) + stat = store.stat_file("full/filename", DUMMY_SPIDER_INFO) + assert stat["checksum"] == "8d777f385d3dfec8815d20f7496026dc" + assert stat["last_modified"] == pytest.approx(time.time(), abs=60) + + def test_stat_missing_file(self, tmp_path: Path) -> None: + store = FSFilesStore(tmp_path) + assert store.stat_file("full/filename", DUMMY_SPIDER_INFO) == {} @pytest.mark.requires_botocore @@ -695,6 +810,59 @@ class TestS3FilesStore: # The call to read does not happen with Stubber assert buffer.method_calls == [mock.call.seek(0)] + @inline_callbacks_test + def test_persist_without_headers(self): + """Without custom headers only the default ones are sent.""" + bucket = "mybucket" + key = "export.csv" + buffer = mock.MagicMock() + + store = S3FilesStore(f"s3://{bucket}/{key}") + from botocore.stub import Stubber # noqa: PLC0415 + + with Stubber(store.s3_client) as stub: + stub.add_response( + "put_object", + expected_params={ + "ACL": S3FilesStore.POLICY, + "Body": buffer, + "Bucket": bucket, + "CacheControl": S3FilesStore.HEADERS["Cache-Control"], + "Key": key, + "Metadata": {}, + }, + service_response={}, + ) + + yield store.persist_file("", buffer, info=DUMMY_SPIDER_INFO) + + stub.assert_no_pending_responses() + + def test_missing_botocore(self): + with ( + mock.patch( + "scrapy.pipelines.files.is_botocore_available", return_value=False + ), + pytest.raises(NotConfigured, match="missing botocore library"), + ): + S3FilesStore("s3://mybucket/key") + + def test_wrong_uri_scheme(self): + with pytest.raises( + ValueError, + match=re.escape( + "Incorrect URI scheme in ftp://mybucket/key, expected 's3'" + ), + ): + S3FilesStore("ftp://mybucket/key") + + def test_unsupported_header(self): + store = S3FilesStore("s3://mybucket/key") + with pytest.raises( + TypeError, match='Header "X-Custom" is not supported by botocore' + ): + store._headers_to_botocore_kwargs({"X-Custom": "value"}) + @inline_callbacks_test def test_stat(self): bucket = "mybucket" @@ -901,6 +1069,28 @@ class TestFTPFileStore: ) assert data == content + @inline_callbacks_test + def test_persist_active_mode(self, monkeypatch: pytest.MonkeyPatch): + data = b"active mode" + path = "full/filename" + monkeypatch.setattr(FTPFilesStore, "FTP_USERNAME", "anonymous") + monkeypatch.setattr(FTPFilesStore, "FTP_PASSWORD", "guest") + monkeypatch.setattr(FTPFilesStore, "USE_ACTIVE_MODE", True) + with MockFTPServer() as ftp_server: + store = FTPFilesStore(ftp_server.url("/")) + yield store.persist_file(path, BytesIO(data), info=DUMMY_SPIDER_INFO) + stat = yield store.stat_file(path, info=DUMMY_SPIDER_INFO) + assert stat["checksum"] == "ff1575649a39a27c13faa0d37c84bab3" + + def test_wrong_uri_scheme(self): + with pytest.raises( + ValueError, + match=re.escape( + "Incorrect URI scheme in http://example.com/, expected 'ftp'" + ), + ): + FTPFilesStore("http://example.com/") + class ItemWithFiles(Item): file_urls = Field() diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py index 1b73dd157..19e61579f 100644 --- a/tests/test_pipeline_images.py +++ b/tests/test_pipeline_images.py @@ -3,20 +3,26 @@ from __future__ import annotations import dataclasses import io import random +import sys from abc import ABC, abstractmethod +from pathlib import Path from shutil import rmtree from tempfile import mkdtemp +from types import SimpleNamespace from typing import Any import attr import pytest from itemadapter import ItemAdapter +from scrapy.exceptions import NotConfigured from scrapy.http import Request, Response from scrapy.item import Field, Item -from scrapy.pipelines.files import GCSFilesStore, S3FilesStore +from scrapy.pipelines.files import GCSFilesStore, S3FilesStore, _md5sum from scrapy.pipelines.images import ImageException, ImagesPipeline from scrapy.utils.test import get_crawler +from tests.utils.decorators import coroutine_test +from tests.utils.media_pipelines import DUMMY_SPIDER_INFO try: from PIL import Image @@ -40,6 +46,11 @@ class TestImagesPipeline: def teardown_method(self): rmtree(self.tempdir) + def test_missing_pillow(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setitem(sys.modules, "PIL", None) + with pytest.raises(NotConfigured, match="requires installing Pillow"): + ImagesPipeline(self.tempdir, crawler=get_crawler()) + def test_file_path(self): file_path = self.pipeline.file_path assert ( @@ -197,6 +208,25 @@ class TestImagesPipeline: assert path == "full/3fd165099d8e71b8a48b2683946e64dbfad8b52d.jpg" assert new_im.getpixel((0, 0)) == (255, 0, 0) + @coroutine_test + async def test_image_downloaded(self) -> None: + """The image and its thumbnails are stored, and the checksum of the + full-size image is returned.""" + self.pipeline.thumbs = {"small": (20, 20)} + _, buf = _create_image("JPEG", "RGB", (50, 50), (0, 0, 0)) + url = "https://dev.mydeco.com/mydeco.gif" + response = Response(url=url, body=buf.getvalue()) + + checksum = await self.pipeline.image_downloaded( + response, Request(url=url), DUMMY_SPIDER_INFO + ) + + buf.seek(0) + assert checksum == _md5sum(buf) + name = "3fd165099d8e71b8a48b2683946e64dbfad8b52d.jpg" + assert Path(self.tempdir, "full", name).read_bytes() == buf.getvalue() + assert Path(self.tempdir, "thumbs", "small", name).exists() + def test_convert_image(self): SIZE = (100, 100) # straight forward case: RGB and JPEG @@ -230,6 +260,24 @@ class TestImagesPipeline: assert converted.mode == "RGB" assert converted.getcolors() == [(10000, (205, 230, 255))] + def test_convert_image_legacy_resampling_filter( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Pillow older than 9.1.0 has Image.ANTIALIAS instead of + Image.Resampling.LANCZOS.""" + # Image.LANCZOS is the only spelling that exists in every supported + # Pillow version, but Pillow defines it dynamically, hence the ignore. + monkeypatch.setattr( + self.pipeline, + "_Image", + SimpleNamespace(ANTIALIAS=Image.LANCZOS), # type: ignore[attr-defined] + ) + im, buf = _create_image("JPEG", "RGB", (100, 100), (0, 127, 255)) + + thumbnail, _ = self.pipeline.convert_image(im, size=(10, 25), response_body=buf) + + assert thumbnail.size == (10, 10) + @pytest.mark.parametrize( "bad_type", [ @@ -581,7 +629,7 @@ class TestImagesPipelineCustomSettings: GCSFilesStore.POLICY = old_policy -def _create_image(format_, *a, **kw): +def _create_image(format_: str, *a: Any, **kw: Any) -> tuple[Image.Image, io.BytesIO]: buf = io.BytesIO() Image.new(*a, **kw).save(buf, format_) buf.seek(0) diff --git a/tests/test_pipeline_media.py b/tests/test_pipeline_media.py index 23c19e4be..ba1c18006 100644 --- a/tests/test_pipeline_media.py +++ b/tests/test_pipeline_media.py @@ -319,6 +319,42 @@ class TestMediaPipeline(TestBaseMediaPipeline): assert self.fingerprint(req1) == self.fingerprint(req2) assert new_item["results"] == [(True, {})] + @coroutine_test + async def test_failures_are_cached_across_multiple_items(self): + self.pipe.LOG_FAILED_RESULTS = False + exc = Exception("foo") + req1 = Request("http://url1", meta={"response": exc}) + new_item = await self.pipe.process_item({"requests": req1}) + assert new_item["results"][0][1].value is exc + + # rsp2 is ignored, the cached failure must be reused because request + # fingerprints are the same + req2 = Request( + req1.url, meta={"response": Response("http://donot.download.me")} + ) + new_item = await self.pipe.process_item({"requests": req2}) + assert new_item["results"][0][0] is False + assert new_item["results"][0][1].value is exc + assert self.pipe._mockcalled.count("media_to_download") == 1 + + @coroutine_test + async def test_cached_failure_calls_errback(self): + """The errback of a request is called for a cached failure as well.""" + self.pipe.LOG_FAILED_RESULTS = False + exc = Exception("foo") + await self.pipe.process_item( + {"requests": Request("http://url1", meta={"response": exc})} + ) + + def errback(failure): + self.pipe._mockcalled.append("request_errback") + return {"recovered": failure.value} + + req = Request("http://url1", errback=errback) + new_item = await self.pipe.process_item({"requests": req}) + assert new_item["results"] == [(True, {"recovered": exc})] + assert self.pipe._mockcalled.count("request_errback") == 1 + @coroutine_test async def test_results_are_cached_for_requests_of_single_item(self): rsp1 = Response("http://url1") @@ -472,6 +508,30 @@ class TestBuildFromCrawler: assert pipe._from_crawler_called +class MediaFailedNonePipeline(MockedMediaPipeline): + def media_failed(self, failure, request, info): + self._mockcalled.append("media_failed") + + +class TestMediaFailedNone(TestBaseMediaPipeline): + """Test what happens when media_failed() neither raises an exception nor + returns a failure.""" + + pipeline_class = MediaFailedNonePipeline + + @coroutine_test + async def test_result_none(self): + req = Request("http://url1", meta={"response": Exception("foo")}) + new_item = await self.pipe.process_item({"requests": req}) + assert new_item["results"] == [(True, None)] + assert self.pipe._mockcalled == [ + "get_media_requests", + "media_to_download", + "media_failed", + "item_completed", + ] + + class MediaFailedFailurePipeline(MockedMediaPipeline): def media_failed(self, failure, request, info): self._mockcalled.append("media_failed") diff --git a/tests/utils/media_pipelines.py b/tests/utils/media_pipelines.py index 283c95940..7e013b797 100644 --- a/tests/utils/media_pipelines.py +++ b/tests/utils/media_pipelines.py @@ -3,6 +3,12 @@ from __future__ import annotations from typing import Any from scrapy.http.request import NO_CALLBACK, Request +from scrapy.pipelines.media import MediaPipeline +from scrapy.utils.spider import DefaultSpider + +# required by persist_file() and stat_file(), but as some stores don't use the argument +# we can pass this singleton to keep type hints correct +DUMMY_SPIDER_INFO = MediaPipeline.SpiderInfo(DefaultSpider()) async def mocked_download_func(request: Request) -> Any: