From 2b04099dc48584baf24b794d0b3df9bd50667108 Mon Sep 17 00:00:00 2001 From: Venelin Stoykov Date: Thu, 18 May 2017 23:38:32 +0300 Subject: [PATCH 1/8] Fixed #368 use specs directly in ProcessedImageField Thanks to @xcono for pointing to solution to the problem --- imagekit/models/fields/__init__.py | 6 +++++- tests/models.py | 11 +++++++++++ tests/test_fields.py | 14 +++++++++++++- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/imagekit/models/fields/__init__.py b/imagekit/models/fields/__init__.py index 04a65460..767374f2 100644 --- a/imagekit/models/fields/__init__.py +++ b/imagekit/models/fields/__init__.py @@ -93,7 +93,7 @@ class ProcessedImageField(models.ImageField, SpecHostField): def __init__(self, processors=None, format=None, options=None, verbose_name=None, name=None, width_field=None, height_field=None, - autoconvert=True, spec=None, spec_id=None, **kwargs): + autoconvert=None, spec=None, spec_id=None, **kwargs): """ The ProcessedImageField constructor accepts all of the arguments that the :class:`django.db.models.ImageField` constructor accepts, as well @@ -101,6 +101,10 @@ def __init__(self, processors=None, format=None, options=None, :class:`imagekit.models.ImageSpecField`. """ + # if spec is not provided then autoconvert will be True by default + if spec is None and autoconvert is None: + autoconvert = True + SpecHost.__init__(self, processors=processors, format=format, options=options, autoconvert=autoconvert, spec=spec, spec_id=spec_id) diff --git a/tests/models.py b/tests/models.py index 5667cad0..0f6bd0c4 100644 --- a/tests/models.py +++ b/tests/models.py @@ -1,10 +1,17 @@ from django.db import models +from imagekit import ImageSpec from imagekit.models import ProcessedImageField from imagekit.models import ImageSpecField from imagekit.processors import Adjust, ResizeToFill, SmartCrop +class Thumbnail(ImageSpec): + processors = [ResizeToFill(100, 60)] + format = 'JPEG' + options = {'quality': 60} + + class ImageModel(models.Model): image = models.ImageField(upload_to='b') @@ -27,6 +34,10 @@ class ProcessedImageFieldModel(models.Model): options={'quality': 90}, upload_to='p') +class ProcessedImageFieldWithSpecModel(models.Model): + processed = ProcessedImageField(spec=Thumbnail, upload_to='p') + + class CountingCacheFileStrategy(object): def __init__(self): self.on_existence_required_count = 0 diff --git a/tests/test_fields.py b/tests/test_fields.py index 6afb6cf7..96e73379 100644 --- a/tests/test_fields.py +++ b/tests/test_fields.py @@ -5,7 +5,9 @@ from imagekit.processors import SmartCrop from nose.tools import eq_ from . import imagegenerators # noqa -from .models import ProcessedImageFieldModel, ImageModel +from .models import (ProcessedImageFieldModel, + ProcessedImageFieldWithSpecModel, + ImageModel) from .utils import get_image_file @@ -19,6 +21,16 @@ def test_model_processedimagefield(): eq_(instance.processed.height, 50) +def test_model_processedimagefield_with_spec(): + instance = ProcessedImageFieldWithSpecModel() + file = File(get_image_file()) + instance.processed.save('whatever.jpeg', file) + instance.save() + + eq_(instance.processed.width, 100) + eq_(instance.processed.height, 60) + + def test_form_processedimagefield(): class TestForm(forms.ModelForm): image = ikforms.ProcessedImageField(spec_id='tests:testform_image', From 755bd34c3e7da9e6d76651d889155b10dcf8e8ca Mon Sep 17 00:00:00 2001 From: Venelin Stoykov Date: Wed, 31 May 2017 11:07:37 +0300 Subject: [PATCH 2/8] In Python 3 files should be opened as binary --- README.rst | 4 ++-- docs/advanced_usage.rst | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.rst b/README.rst index 7d40b053..e6d91036 100644 --- a/README.rst +++ b/README.rst @@ -144,7 +144,7 @@ on, or what should be done with the result; that's up to you: .. code-block:: python - source_file = open('/path/to/myimage.jpg') + source_file = open('/path/to/myimage.jpg', 'rb') image_generator = Thumbnail(source=source_file) result = image_generator.generate() @@ -159,7 +159,7 @@ example, if you wanted to save it to disk: .. code-block:: python - dest = open('/path/to/dest.jpg', 'w') + dest = open('/path/to/dest.jpg', 'wb') dest.write(result.read()) dest.close() diff --git a/docs/advanced_usage.rst b/docs/advanced_usage.rst index bb635e76..37458c46 100644 --- a/docs/advanced_usage.rst +++ b/docs/advanced_usage.rst @@ -163,7 +163,7 @@ A simple example of a custom source group class is as follows: def files(self): os.chdir(self.dir) for name in glob.glob('*.jpg'): - yield open(name) + yield open(name, 'rb') Instances of this class could then be registered with one or more spec id: From 2e1b574486d3dce2924ab1a95b999133799e0704 Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Mon, 24 Jul 2017 13:41:54 +0100 Subject: [PATCH 3/8] README - use Python 3 print function It's 2017!!! --- README.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.rst b/README.rst index e6d91036..d797561e 100644 --- a/README.rst +++ b/README.rst @@ -70,8 +70,8 @@ your model class: options={'quality': 60}) profile = Profile.objects.all()[0] - print profile.avatar_thumbnail.url # > /media/CACHE/images/982d5af84cddddfd0fbf70892b4431e4.jpg - print profile.avatar_thumbnail.width # > 100 + print(profile.avatar_thumbnail.url) # > /media/CACHE/images/982d5af84cddddfd0fbf70892b4431e4.jpg + print(profile.avatar_thumbnail.width) # > 100 As you can probably tell, ImageSpecFields work a lot like Django's ImageFields. The difference is that they're automatically generated by @@ -97,8 +97,8 @@ class: options={'quality': 60}) profile = Profile.objects.all()[0] - print profile.avatar_thumbnail.url # > /media/avatars/MY-avatar.jpg - print profile.avatar_thumbnail.width # > 100 + print(profile.avatar_thumbnail.url) # > /media/avatars/MY-avatar.jpg + print(profile.avatar_thumbnail.width) # > 100 This is pretty similar to our previous example. We don't need to specify a "source" any more since we're not processing another image field, but we do need From a153812add3f745e8de39630de509ee9b1dd54c6 Mon Sep 17 00:00:00 2001 From: Yuri Kanivetsky Date: Tue, 29 Aug 2017 11:27:10 +0300 Subject: [PATCH 4/8] generateimages: fix taking arguments --- imagekit/management/commands/generateimages.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/imagekit/management/commands/generateimages.py b/imagekit/management/commands/generateimages.py index 0d9cae5c..4983af3f 100644 --- a/imagekit/management/commands/generateimages.py +++ b/imagekit/management/commands/generateimages.py @@ -12,13 +12,15 @@ class Command(BaseCommand): "a:*:c" will match "a:b:c", but not "a:b:x:c", whereas "a:**:c" will match both. Subsegments are always matched, so "a" will match "a" as well as "a:b" and "a:b:c".""") - args = '[generator_ids]' + + def add_arguments(self, parser): + parser.add_argument('generator_id', nargs='*', help=':: for model specs') def handle(self, *args, **options): generators = generator_registry.get_ids() - if args: - patterns = self.compile_patterns(args) + if options['generator_id']: + patterns = self.compile_patterns(options['generator_id']) generators = (id for id in generators if any(p.match(id) for p in patterns)) for generator_id in generators: From de3047e73d813ec9a2e96381a3bda5a2cf19cd78 Mon Sep 17 00:00:00 2001 From: Yuri Kanivetsky Date: Tue, 12 Sep 2017 20:44:22 +0300 Subject: [PATCH 5/8] Make generateimages support pre Django 1.8 versions --- imagekit/management/commands/generateimages.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/imagekit/management/commands/generateimages.py b/imagekit/management/commands/generateimages.py index 4983af3f..2addfd31 100644 --- a/imagekit/management/commands/generateimages.py +++ b/imagekit/management/commands/generateimages.py @@ -12,6 +12,7 @@ class Command(BaseCommand): "a:*:c" will match "a:b:c", but not "a:b:x:c", whereas "a:**:c" will match both. Subsegments are always matched, so "a" will match "a" as well as "a:b" and "a:b:c".""") + args = '[generator_ids]' def add_arguments(self, parser): parser.add_argument('generator_id', nargs='*', help=':: for model specs') @@ -19,8 +20,9 @@ def add_arguments(self, parser): def handle(self, *args, **options): generators = generator_registry.get_ids() - if options['generator_id']: - patterns = self.compile_patterns(options['generator_id']) + generator_ids = options['generator_id'] if 'generator_id' in options else args + if generator_ids: + patterns = self.compile_patterns(generator_ids) generators = (id for id in generators if any(p.match(id) for p in patterns)) for generator_id in generators: From d80f426d3cbb24bbdb0d92cba3ba7f9484767104 Mon Sep 17 00:00:00 2001 From: Roman Gorbil Date: Tue, 10 Oct 2017 17:39:46 +0700 Subject: [PATCH 6/8] Fix `ImageCacheFile.__repr__` to not send signals Cachefile strategy may be configured to generate file when file existance required. To generate images, async backends passes `ImageCacheFile` instance to worker. Both celery and RQ calls `__repr__` method for each argument to enque call. And if `__repr__` of object will send `existnace_required` signal, we will get endless recursion. Issue: #434 --- imagekit/cachefiles/__init__.py | 6 ++++++ tests/test_cachefiles.py | 26 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/imagekit/cachefiles/__init__.py b/imagekit/cachefiles/__init__.py index 2ba5cb8c..594df57c 100644 --- a/imagekit/cachefiles/__init__.py +++ b/imagekit/cachefiles/__init__.py @@ -3,6 +3,7 @@ from django.core.files import File from django.core.files.images import ImageFile from django.utils.functional import SimpleLazyObject +from django.utils.encoding import smart_str from ..files import BaseIKFile from ..registry import generator_registry from ..signals import content_required, existence_required @@ -149,6 +150,11 @@ def __nonzero__(self): # Python 2 compatibility return self.__bool__() + def __repr__(self): + return smart_str("<%s: %s>" % ( + self.__class__.__name__, self if self.name else "None") + ) + class LazyImageCacheFile(SimpleLazyObject): def __init__(self, generator_id, *args, **kwargs): diff --git a/tests/test_cachefiles.py b/tests/test_cachefiles.py index 8a93e716..ad01dda3 100644 --- a/tests/test_cachefiles.py +++ b/tests/test_cachefiles.py @@ -1,3 +1,4 @@ +import mock from django.conf import settings from hashlib import md5 from imagekit.cachefiles import ImageCacheFile, LazyImageCacheFile @@ -48,6 +49,31 @@ def test_no_source_error(): file.generate() +def test_repr_does_not_send_existence_required(): + """ + Ensure that `__repr__` method does not send `existance_required` signal + + Cachefile strategy may be configured to generate file on + `existance_required`. + To generate images, backend passes `ImageCacheFile` instance to worker. + Both celery and RQ calls `__repr__` method for each argument to enque call. + And if `__repr__` of object will send this signal, we will get endless + recursion + + """ + with mock.patch('imagekit.cachefiles.existence_required') as signal: + # import here to apply mock + from imagekit.cachefiles import ImageCacheFile + + spec = TestSpec(source=get_unique_image_file()) + file = ImageCacheFile( + spec, + cachefile_backend=DummyAsyncCacheFileBackend() + ) + file.__repr__() + eq_(signal.send.called, False) + + def test_memcached_cache_key(): """ Ensure the default cachefile backend is sanitizing its cache key for From 6ee931398f3003c5e7a1a35b0ca8a99c49ac3012 Mon Sep 17 00:00:00 2001 From: Venelin Stoykov Date: Fri, 17 Nov 2017 18:31:23 +0200 Subject: [PATCH 7/8] Do not leak open files after generation --- imagekit/specs/__init__.py | 27 +++++++++++++++------------ tests/test_closing_fieldfiles.py | 25 +++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 12 deletions(-) create mode 100644 tests/test_closing_fieldfiles.py diff --git a/imagekit/specs/__init__.py b/imagekit/specs/__init__.py index 1ef298e1..d9339f37 100644 --- a/imagekit/specs/__init__.py +++ b/imagekit/specs/__init__.py @@ -143,23 +143,26 @@ def generate(self): raise MissingSource("The spec '%s' has no source file associated" " with it." % self) - file_opened_locally = False # TODO: Move into a generator base class # TODO: Factor out a generate_image function so you can create a generator and only override the PIL.Image creating part. (The tricky part is how to deal with original_format since generator base class won't have one.) - try: - img = open_image(self.source) - except ValueError: - # Re-open the file -- https://code.djangoproject.com/ticket/13750 + closed = self.source.closed + if closed: + # Django file object should know how to reopen itself if it was closed + # https://code.djangoproject.com/ticket/13750 self.source.open() - file_opened_locally = True - img = open_image(self.source) - new_image = process_image(img, processors=self.processors, - format=self.format, autoconvert=self.autoconvert, - options=self.options) - if file_opened_locally: - self.source.close() + try: + img = open_image(self.source) + new_image = process_image(img, + processors=self.processors, + format=self.format, + autoconvert=self.autoconvert, + options=self.options) + finally: + if closed: + # We need to close the file if it was opened by us + self.source.close() return new_image diff --git a/tests/test_closing_fieldfiles.py b/tests/test_closing_fieldfiles.py new file mode 100644 index 00000000..d8693c39 --- /dev/null +++ b/tests/test_closing_fieldfiles.py @@ -0,0 +1,25 @@ +from nose.tools import assert_false, assert_true + +from .models import Thumbnail +from .utils import create_photo + + +def test_do_not_leak_open_files(): + instance = create_photo('leak-test.jpg') + source_file = instance.original_image + # Ensure the FieldFile is closed before generation + source_file.close() + image_generator = Thumbnail(source=source_file) + image_generator.generate() + assert_true(source_file.closed) + + +def test_do_not_close_open_files_after_generate(): + instance = create_photo('do-not-close-test.jpg') + source_file = instance.original_image + # Ensure the FieldFile is opened before generation + source_file.open() + image_generator = Thumbnail(source=source_file) + image_generator.generate() + assert_false(source_file.closed) + source_file.close() From ea66e3d10d3d8d8cf221486b7b1df5d62f9c29f9 Mon Sep 17 00:00:00 2001 From: Venelin Stoykov Date: Mon, 20 Nov 2017 10:24:12 +0200 Subject: [PATCH 8/8] Bump version to 4.0.2 --- imagekit/pkgmeta.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/imagekit/pkgmeta.py b/imagekit/pkgmeta.py index 6c9eefe4..afdaca40 100644 --- a/imagekit/pkgmeta.py +++ b/imagekit/pkgmeta.py @@ -1,5 +1,5 @@ __title__ = 'django-imagekit' __author__ = 'Matthew Tretter, Venelin Stoykov, Eric Eldredge, Bryan Veloso, Greg Newman, Chris Drackett, Justin Driscoll' -__version__ = '4.0.1' +__version__ = '4.0.2' __license__ = 'BSD' __all__ = ['__title__', '__author__', '__version__', '__license__']