diff --git a/README.rst b/README.rst index 7d40b053..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 @@ -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 f02e122c..f04a3945 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: 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/imagekit/management/commands/generateimages.py b/imagekit/management/commands/generateimages.py index 0d9cae5c..2addfd31 100644 --- a/imagekit/management/commands/generateimages.py +++ b/imagekit/management/commands/generateimages.py @@ -14,11 +14,15 @@ class Command(BaseCommand): 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) + 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: 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/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__'] 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/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_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 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() 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',