diff --git a/.gitignore b/.gitignore index 40b09c6..575a800 100644 --- a/.gitignore +++ b/.gitignore @@ -10,4 +10,5 @@ dist /tests/media/* !/tests/media/lenna.png /venv +/venv3 /.env diff --git a/MANIFEST.in b/MANIFEST.in index 0adbc50..94be0c5 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -3,3 +3,4 @@ include LICENSE include README.rst recursive-include docs * recursive-include imagekit/templates * +prune tests diff --git a/README.rst b/README.rst index e098bae..80e0e75 100644 --- a/README.rst +++ b/README.rst @@ -136,7 +136,7 @@ particularly when the processing being done depends on user input. format = 'JPEG' options = {'quality': 60} -It's probaby not surprising that this class is capable of processing an image +It's probably not surprising that this class is capable of processing an image in the exact same way as our ImageSpecField above. However, unlike with the image spec model field, this class doesn't define what source the spec is acting on, or what should be done with the result; that's up to you: diff --git a/imagekit/cachefiles/__init__.py b/imagekit/cachefiles/__init__.py index 34892ec..2ba5cb8 100644 --- a/imagekit/cachefiles/__init__.py +++ b/imagekit/cachefiles/__init__.py @@ -128,7 +128,14 @@ class ImageCacheFile(BaseIKFile, ImageFile): # Dispatch the existence_required signal before checking to see if the # file exists. This gives the strategy a chance to create the file. existence_required.send(sender=self, file=self) - return self.cachefile_backend.exists(self) + + try: + check = self.cachefile_strategy.should_verify_existence(self) + except AttributeError: + # All synchronous backends should have created the file as part of + # `existence_required` if they wanted to. + check = getattr(self.cachefile_backend, 'is_async', False) + return self.cachefile_backend.exists(self) if check else True def __getstate__(self): state = copy(self.__dict__) diff --git a/imagekit/cachefiles/backends.py b/imagekit/cachefiles/backends.py index 25ce43f..1512f51 100644 --- a/imagekit/cachefiles/backends.py +++ b/imagekit/cachefiles/backends.py @@ -121,6 +121,8 @@ class BaseAsync(Simple): """ Base class for cache file backends that generate files asynchronously. """ + is_async = True + def generate(self, file, force=False): # Schedule the file for generation, unless we know for sure we don't # need to. If an already-generated file sneaks through, that's okay; diff --git a/imagekit/cachefiles/strategies.py b/imagekit/cachefiles/strategies.py index 98f712e..bcd8211 100644 --- a/imagekit/cachefiles/strategies.py +++ b/imagekit/cachefiles/strategies.py @@ -29,6 +29,9 @@ class Optimistic(object): def on_source_saved(self, file): file.generate() + def should_verify_existence(self, file): + return False + class DictStrategy(object): def __init__(self, callbacks): diff --git a/imagekit/pkgmeta.py b/imagekit/pkgmeta.py index 81478c3..b7d0e81 100644 --- a/imagekit/pkgmeta.py +++ b/imagekit/pkgmeta.py @@ -1,5 +1,5 @@ __title__ = 'django-imagekit' __author__ = 'Matthew Tretter, Eric Eldredge, Bryan Veloso, Greg Newman, Chris Drackett, Justin Driscoll' -__version__ = '3.2.2' +__version__ = '3.2.3' __license__ = 'BSD' __all__ = ['__title__', '__author__', '__version__', '__license__'] diff --git a/imagekit/specs/sourcegroups.py b/imagekit/specs/sourcegroups.py index 2008f59..d8ea3a7 100644 --- a/imagekit/specs/sourcegroups.py +++ b/imagekit/specs/sourcegroups.py @@ -72,18 +72,19 @@ class ModelSignalRouter(object): """ self.init_instance(instance) - instance._ik['source_hashes'] = dict((attname, hash(file_field)) - for attname, file_field in self.get_field_dict(instance).items()) + instance._ik['source_hashes'] = dict( + (attname, hash(getattr(instance, attname))) + for attname in self.get_source_fields(instance)) return instance._ik['source_hashes'] - def get_field_dict(self, instance): + def get_source_fields(self, instance): """ - Returns the source fields for the given instance, in a dictionary whose - keys are the field names and values are the fields themselves. + Returns a list of the source fields for the given instance. """ - return dict((src.image_field, getattr(instance, src.image_field)) for - src in self._source_groups if isinstance(instance, src.model_class)) + return set(src.image_field + for src in self._source_groups + if isinstance(instance, src.model_class)) @ik_model_receiver def post_save_receiver(self, sender, instance=None, created=False, raw=False, **kwargs): @@ -91,14 +92,22 @@ class ModelSignalRouter(object): self.init_instance(instance) old_hashes = instance._ik.get('source_hashes', {}).copy() new_hashes = self.update_source_hashes(instance) - for attname, file in self.get_field_dict(instance).items(): - if file and old_hashes[attname] != new_hashes[attname]: + for attname in self.get_source_fields(instance): + file = getattr(instance, attname) + if file and old_hashes.get(attname) != new_hashes[attname]: self.dispatch_signal(source_saved, file, sender, instance, attname) @ik_model_receiver def post_init_receiver(self, sender, instance=None, **kwargs): - self.update_source_hashes(instance) + self.init_instance(instance) + source_fields = self.get_source_fields(instance) + local_fields = dict((field.name, field) + for field in instance._meta.local_fields + if field.name in source_fields) + instance._ik['source_hashes'] = dict( + (attname, hash(file_field)) + for attname, file_field in local_fields.items()) def dispatch_signal(self, signal, file, model_class, instance, attname): """ diff --git a/setup.py b/setup.py index 9a65738..8f60701 100644 --- a/setup.py +++ b/setup.py @@ -48,6 +48,7 @@ setup( 'nose-progressive==1.5', 'django-nose==1.2', 'Pillow<3.0', + 'mock==1.0.1', ], test_suite='testrunner.run_tests', install_requires=[ diff --git a/tests/test_no_extra_queries.py b/tests/test_no_extra_queries.py new file mode 100644 index 0000000..89c68cb --- /dev/null +++ b/tests/test_no_extra_queries.py @@ -0,0 +1,16 @@ +from nose.tools import assert_false +from mock import Mock, PropertyMock, patch +from .models import Photo + + +def test_dont_access_source(): + """ + Touching the source may trigger an unneeded query. + See + + """ + pmock = PropertyMock() + pmock.__get__ = Mock() + with patch.object(Photo, 'original_image', pmock): + photo = Photo() # noqa + assert_false(pmock.__get__.called) diff --git a/tests/test_optimistic_strategy.py b/tests/test_optimistic_strategy.py new file mode 100644 index 0000000..18b8f3f --- /dev/null +++ b/tests/test_optimistic_strategy.py @@ -0,0 +1,49 @@ +from nose.tools import assert_true, assert_false +from imagekit.cachefiles import ImageCacheFile +from mock import Mock +from .utils import create_image +from django.core.files.storage import FileSystemStorage +from imagekit.cachefiles.backends import Simple as SimpleCFBackend +from imagekit.cachefiles.strategies import Optimistic as OptimisticStrategy + + +class ImageGenerator(object): + def generate(self): + return create_image() + + def get_hash(self): + return 'abc123' + + +def get_image_cache_file(): + storage = Mock(FileSystemStorage) + backend = SimpleCFBackend() + strategy = OptimisticStrategy() + generator = ImageGenerator() + return ImageCacheFile(generator, storage=storage, + cachefile_backend=backend, + cachefile_strategy=strategy) + + +def test_no_io_on_bool(): + """ + When checking the truthiness of an ImageCacheFile, the storage shouldn't + peform IO operations. + + """ + file = get_image_cache_file() + bool(file) + assert_false(file.storage.exists.called) + assert_false(file.storage.open.called) + + +def test_no_io_on_url(): + """ + When getting the URL of an ImageCacheFile, the storage shouldn't be + checked. + + """ + file = get_image_cache_file() + file.url + assert_false(file.storage.exists.called) + assert_false(file.storage.open.called) diff --git a/tests/utils.py b/tests/utils.py index 5a9f1c5..b6c4952 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -77,5 +77,7 @@ class DummyAsyncCacheFileBackend(Simple): A cache file backend meant to simulate async generation. """ + is_async = True + def generate(self, file, force=False): pass