diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py index 4d59436ab..6a28a3b87 100644 --- a/scrapy/pipelines/images.py +++ b/scrapy/pipelines/images.py @@ -17,7 +17,7 @@ from scrapy.pipelines.files import FileException, FilesPipeline # TODO: from scrapy.pipelines.media import MediaPipeline from scrapy.settings import Settings from scrapy.utils.misc import md5sum -from scrapy.utils.python import to_bytes +from scrapy.utils.python import get_func_args, to_bytes class NoimagesDrop(DropItem): @@ -92,6 +92,8 @@ class ImagesPipeline(FilesPipeline): resolve('IMAGES_THUMBS'), self.THUMBS ) + self._deprecated_convert_image = None + @classmethod def from_settings(cls, settings): s3store = cls.STORE_SCHEMES['s3'] @@ -142,15 +144,33 @@ class ImagesPipeline(FilesPipeline): f"({width}x{height} < " f"{self.min_width}x{self.min_height})") - image, buf = self.convert_image(orig_image) + if self._deprecated_convert_image is None: + self._deprecated_convert_image = 'response_body' not in get_func_args(self.convert_image) + if self._deprecated_convert_image: + warnings.warn(f'{self.__class__.__name__}.convert_image() method overriden in a deprecated way, ' + 'overriden method does not accept response_body argument.', + category=ScrapyDeprecationWarning) + + if self._deprecated_convert_image: + image, buf = self.convert_image(orig_image) + else: + image, buf = self.convert_image(orig_image, response_body=BytesIO(response.body)) yield path, image, buf for thumb_id, size in self.thumbs.items(): thumb_path = self.thumb_path(request, thumb_id, response=response, info=info, item=item) - thumb_image, thumb_buf = self.convert_image(image, size) + if self._deprecated_convert_image: + thumb_image, thumb_buf = self.convert_image(image, size) + else: + thumb_image, thumb_buf = self.convert_image(image, size, buf) yield thumb_path, thumb_image, thumb_buf - def convert_image(self, image, size=None): + def convert_image(self, image, size=None, response_body=None): + if response_body is None: + warnings.warn(f'{self.__class__.__name__}.convert_image() method called in a deprecated way, ' + 'method called without response_body argument.', + category=ScrapyDeprecationWarning, stacklevel=2) + if image.format == 'PNG' and image.mode == 'RGBA': background = self._Image.new('RGBA', image.size, (255, 255, 255)) background.paste(image, image) @@ -173,6 +193,8 @@ class ImagesPipeline(FilesPipeline): except AttributeError: resampling_filter = self._Image.ANTIALIAS image.thumbnail(size, resampling_filter) + elif response_body is not None and image.format == 'JPEG': + return image, response_body buf = BytesIO() image.save(buf, 'JPEG') diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py index c189d08bf..f98d40fda 100644 --- a/tests/test_pipeline_images.py +++ b/tests/test_pipeline_images.py @@ -2,9 +2,10 @@ import dataclasses import hashlib import io import random +import warnings from shutil import rmtree from tempfile import mkdtemp -from warnings import catch_warnings +from unittest.mock import patch import attr from itemadapter import ItemAdapter @@ -13,7 +14,7 @@ from twisted.trial import unittest from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Request, Response from scrapy.item import Field, Item -from scrapy.pipelines.images import ImagesPipeline, NoimagesDrop +from scrapy.pipelines.images import ImageException, ImagesPipeline, NoimagesDrop from scrapy.settings import Settings from scrapy.utils.python import to_bytes @@ -104,32 +105,143 @@ class ImagesPipelineTestCase(unittest.TestCase): request = Request("http://example.com") self.assertEqual(thumb_path(request, 'small', item=item), 'thumb/small/path-to-store-file') - def test_convert_image(self): + def test_get_images_exception(self): + self.pipeline.min_width = 100 + self.pipeline.min_height = 100 + + _, buf1 = _create_image('JPEG', 'RGB', (50, 50), (0, 0, 0)) + _, buf2 = _create_image('JPEG', 'RGB', (150, 50), (0, 0, 0)) + _, buf3 = _create_image('JPEG', 'RGB', (50, 150), (0, 0, 0)) + + resp1 = Response(url="https://dev.mydeco.com/mydeco.gif", body=buf1.getvalue()) + resp2 = Response(url="https://dev.mydeco.com/mydeco.gif", body=buf2.getvalue()) + resp3 = Response(url="https://dev.mydeco.com/mydeco.gif", body=buf3.getvalue()) + req = Request(url="https://dev.mydeco.com/mydeco.gif") + + with self.assertRaises(ImageException): + next(self.pipeline.get_images(response=resp1, request=req, info=object())) + with self.assertRaises(ImageException): + next(self.pipeline.get_images(response=resp2, request=req, info=object())) + with self.assertRaises(ImageException): + next(self.pipeline.get_images(response=resp3, request=req, info=object())) + + def test_get_images_new(self): + self.pipeline.min_width = 0 + self.pipeline.min_height = 0 + self.pipeline.thumbs = {'small': (20, 20)} + + orig_im, buf = _create_image('JPEG', 'RGB', (50, 50), (0, 0, 0)) + orig_thumb, orig_thumb_buf = _create_image('JPEG', 'RGB', (20, 20), (0, 0, 0)) + resp = Response(url="https://dev.mydeco.com/mydeco.gif", body=buf.getvalue()) + req = Request(url="https://dev.mydeco.com/mydeco.gif") + + get_images_gen = self.pipeline.get_images(response=resp, request=req, info=object()) + + path, new_im, new_buf = next(get_images_gen) + self.assertEqual(path, 'full/3fd165099d8e71b8a48b2683946e64dbfad8b52d.jpg') + self.assertEqual(orig_im, new_im) + self.assertEqual(buf.getvalue(), new_buf.getvalue()) + + thumb_path, thumb_img, thumb_buf = next(get_images_gen) + self.assertEqual(thumb_path, 'thumbs/small/3fd165099d8e71b8a48b2683946e64dbfad8b52d.jpg') + self.assertEqual(thumb_img, thumb_img) + self.assertEqual(orig_thumb_buf.getvalue(), thumb_buf.getvalue()) + + def test_get_images_old(self): + self.pipeline.thumbs = {'small': (20, 20)} + orig_im, buf = _create_image('JPEG', 'RGB', (50, 50), (0, 0, 0)) + resp = Response(url="https://dev.mydeco.com/mydeco.gif", body=buf.getvalue()) + req = Request(url="https://dev.mydeco.com/mydeco.gif") + + def overridden_convert_image(image, size=None): + im, buf = _create_image('JPEG', 'RGB', (50, 50), (0, 0, 0)) + return im, buf + + with patch.object(self.pipeline, 'convert_image', overridden_convert_image): + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter('always') + get_images_gen = self.pipeline.get_images(response=resp, request=req, info=object()) + path, new_im, new_buf = next(get_images_gen) + self.assertEqual(path, 'full/3fd165099d8e71b8a48b2683946e64dbfad8b52d.jpg') + self.assertEqual(orig_im.mode, new_im.mode) + self.assertEqual(orig_im.getcolors(), new_im.getcolors()) + self.assertEqual(buf.getvalue(), new_buf.getvalue()) + + thumb_path, thumb_img, thumb_buf = next(get_images_gen) + self.assertEqual(thumb_path, 'thumbs/small/3fd165099d8e71b8a48b2683946e64dbfad8b52d.jpg') + self.assertEqual(orig_im.mode, thumb_img.mode) + self.assertEqual(orig_im.getcolors(), thumb_img.getcolors()) + self.assertEqual(buf.getvalue(), thumb_buf.getvalue()) + + expected_warning_msg = ('.convert_image() method overriden in a deprecated way, ' + 'overriden method does not accept response_body argument.') + self.assertEqual(len([warning for warning in w if expected_warning_msg in str(warning.message)]), 1) + + def test_convert_image_old(self): + # tests for old API + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter('always') + SIZE = (100, 100) + # straigh forward case: RGB and JPEG + COLOUR = (0, 127, 255) + im, _ = _create_image('JPEG', 'RGB', SIZE, COLOUR) + converted, _ = self.pipeline.convert_image(im) + self.assertEqual(converted.mode, 'RGB') + self.assertEqual(converted.getcolors(), [(10000, COLOUR)]) + + # check that thumbnail keep image ratio + thumbnail, _ = self.pipeline.convert_image(converted, size=(10, 25)) + self.assertEqual(thumbnail.mode, 'RGB') + self.assertEqual(thumbnail.size, (10, 10)) + + # transparency case: RGBA and PNG + COLOUR = (0, 127, 255, 50) + im, _ = _create_image('PNG', 'RGBA', SIZE, COLOUR) + converted, _ = self.pipeline.convert_image(im) + self.assertEqual(converted.mode, 'RGB') + self.assertEqual(converted.getcolors(), [(10000, (205, 230, 255))]) + + # transparency case with palette: P and PNG + COLOUR = (0, 127, 255, 50) + im, _ = _create_image('PNG', 'RGBA', SIZE, COLOUR) + im = im.convert('P') + converted, _ = self.pipeline.convert_image(im) + self.assertEqual(converted.mode, 'RGB') + self.assertEqual(converted.getcolors(), [(10000, (205, 230, 255))]) + + # ensure that we recieved deprecation warnings + expected_warning_msg = '.convert_image() method called in a deprecated way' + self.assertTrue(len([warning for warning in w if expected_warning_msg in str(warning.message)]) == 4) + + def test_convert_image_new(self): + # tests for new API SIZE = (100, 100) # straigh forward case: RGB and JPEG COLOUR = (0, 127, 255) - im = _create_image('JPEG', 'RGB', SIZE, COLOUR) - converted, _ = self.pipeline.convert_image(im) + im, buf = _create_image('JPEG', 'RGB', SIZE, COLOUR) + converted, converted_buf = self.pipeline.convert_image(im, response_body=buf) self.assertEqual(converted.mode, 'RGB') self.assertEqual(converted.getcolors(), [(10000, COLOUR)]) + # check that we don't convert JPEGs again + self.assertEqual(converted_buf, buf) # check that thumbnail keep image ratio - thumbnail, _ = self.pipeline.convert_image(converted, size=(10, 25)) + thumbnail, _ = self.pipeline.convert_image(converted, size=(10, 25), response_body=converted_buf) self.assertEqual(thumbnail.mode, 'RGB') self.assertEqual(thumbnail.size, (10, 10)) # transparency case: RGBA and PNG COLOUR = (0, 127, 255, 50) - im = _create_image('PNG', 'RGBA', SIZE, COLOUR) - converted, _ = self.pipeline.convert_image(im) + im, buf = _create_image('PNG', 'RGBA', SIZE, COLOUR) + converted, _ = self.pipeline.convert_image(im, response_body=buf) self.assertEqual(converted.mode, 'RGB') self.assertEqual(converted.getcolors(), [(10000, (205, 230, 255))]) # transparency case with palette: P and PNG COLOUR = (0, 127, 255, 50) - im = _create_image('PNG', 'RGBA', SIZE, COLOUR) + im, buf = _create_image('PNG', 'RGBA', SIZE, COLOUR) im = im.convert('P') - converted, _ = self.pipeline.convert_image(im) + converted, _ = self.pipeline.convert_image(im, response_body=buf) self.assertEqual(converted.mode, 'RGB') self.assertEqual(converted.getcolors(), [(10000, (205, 230, 255))]) @@ -422,23 +534,23 @@ class NoimagesDropTestCase(unittest.TestCase): def test_deprecation_warning(self): arg = str() - with catch_warnings(record=True) as warnings: + with warnings.catch_warnings(record=True) as w: NoimagesDrop(arg) - self.assertEqual(len(warnings), 1) - self.assertEqual(warnings[0].category, ScrapyDeprecationWarning) - with catch_warnings(record=True) as warnings: + self.assertEqual(len(w), 1) + self.assertEqual(w[0].category, ScrapyDeprecationWarning) + with warnings.catch_warnings(record=True) as w: class SubclassedNoimagesDrop(NoimagesDrop): pass SubclassedNoimagesDrop(arg) - self.assertEqual(len(warnings), 1) - self.assertEqual(warnings[0].category, ScrapyDeprecationWarning) + self.assertEqual(len(w), 1) + self.assertEqual(w[0].category, ScrapyDeprecationWarning) def _create_image(format, *a, **kw): buf = io.BytesIO() Image.new(*a, **kw).save(buf, format) buf.seek(0) - return Image.open(buf) + return Image.open(buf), buf if __name__ == "__main__":