mirror of https://github.com/scrapy/scrapy.git
Merge pull request #4753 from drs-11/3689-update
Do not recode JPEG files in imagepipelines
This commit is contained in:
Andrey Rahmatullin
GitHub
commit
c56caaf0b7
|
|
@ -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')
|
||||
|
|
|
|||
|
|
@ -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__":
|
||||
|
|
|
|||
Loading…
Reference in New Issue