From a8855d4c27ce0df8cf8c0b40d2c1c368b2befebe Mon Sep 17 00:00:00 2001 From: Eric Eldredge Date: Wed, 23 Jan 2013 22:46:57 -0500 Subject: [PATCH 01/10] Change spec/source registry to generator/cacheable --- imagekit/generatorlibrary.py | 2 +- imagekit/imagecache/strategies.py | 12 +- .../management/commands/warmimagecache.py | 34 ++--- imagekit/models/fields/__init__.py | 4 +- imagekit/registry.py | 116 +++++++++--------- imagekit/signals.py | 6 +- imagekit/specs/__init__.py | 2 +- imagekit/specs/sourcegroups.py | 18 +-- tests/imagespecs.py | 4 +- 9 files changed, 104 insertions(+), 94 deletions(-) diff --git a/imagekit/generatorlibrary.py b/imagekit/generatorlibrary.py index bd9da9a..977e960 100644 --- a/imagekit/generatorlibrary.py +++ b/imagekit/generatorlibrary.py @@ -10,4 +10,4 @@ class Thumbnail(ImageSpec): super(Thumbnail, self).__init__(**kwargs) -register.spec('ik:thumbnail', Thumbnail) +register.generator('ik:thumbnail', Thumbnail) diff --git a/imagekit/imagecache/strategies.py b/imagekit/imagecache/strategies.py index 630b77c..a0b8f76 100644 --- a/imagekit/imagecache/strategies.py +++ b/imagekit/imagecache/strategies.py @@ -14,19 +14,19 @@ class JustInTime(object): class Optimistic(object): """ - A caching strategy that acts immediately when the source file chages and - assumes that the cache files will not be removed (i.e. doesn't revalidate - on access). + A caching strategy that acts immediately when the cacheable file changes + and assumes that the cache files will not be removed (i.e. doesn't + revalidate on access). """ - def on_source_created(self, file): + def on_cacheable_created(self, file): validate_now(file) - def on_source_deleted(self, file): + def on_cacheable_deleted(self, file): clear_now(file) - def on_source_changed(self, file): + def on_cacheable_changed(self, file): validate_now(file) diff --git a/imagekit/management/commands/warmimagecache.py b/imagekit/management/commands/warmimagecache.py index 6c9822d..42a005d 100644 --- a/imagekit/management/commands/warmimagecache.py +++ b/imagekit/management/commands/warmimagecache.py @@ -1,35 +1,35 @@ from django.core.management.base import BaseCommand import re from ...files import GeneratedImageCacheFile -from ...registry import generator_registry, source_group_registry +from ...registry import generator_registry, cacheable_registry class Command(BaseCommand): - help = ('Warm the image cache for the specified specs (or all specs if none' - ' was provided). Simple wildcard matching (using asterisks) is' - ' supported.') - args = '[spec_ids]' + help = ('Warm the image cache for the specified generators' + ' (or all generators if none was provided).' + ' Simple wildcard matching (using asterisks) is supported.') + args = '[generator_ids]' def handle(self, *args, **options): - specs = generator_registry.get_ids() + generators = generator_registry.get_ids() if args: patterns = self.compile_patterns(args) - specs = (id for id in specs if any(p.match(id) for p in patterns)) + generators = (id for id in generators if any(p.match(id) for p in patterns)) - for spec_id in specs: - self.stdout.write('Validating spec: %s\n' % spec_id) - for source_group in source_group_registry.get(spec_id): - for source in source_group.files(): - if source: - spec = generator_registry.get(spec_id, source=source) # TODO: HINTS! (Probably based on source, so this will need to be moved into loop below.) - self.stdout.write(' %s\n' % source) + for generator_id in generators: + self.stdout.write('Validating generator: %s\n' % generator_id) + for cacheables in cacheable_registry.get(generator_id): + for cacheable in cacheables.files(): + if cacheable: + generator = generator_registry.get(generator_id, cacheable=cacheable) # TODO: HINTS! (Probably based on cacheable, so this will need to be moved into loop below.) + self.stdout.write(' %s\n' % cacheable) try: # TODO: Allow other validation actions through command option - GeneratedImageCacheFile(spec).validate() + GeneratedImageCacheFile(generator).validate() except Exception, err: # TODO: How should we handle failures? Don't want to error, but should call it out more than this. self.stdout.write(' FAILED: %s\n' % err) - def compile_patterns(self, spec_ids): - return [re.compile('%s$' % '.*'.join(re.escape(part) for part in id.split('*'))) for id in spec_ids] + def compile_patterns(self, generator_ids): + return [re.compile('%s$' % '.*'.join(re.escape(part) for part in id.split('*'))) for id in generator_ids] diff --git a/imagekit/models/fields/__init__.py b/imagekit/models/fields/__init__.py index 131ff33..449faac 100644 --- a/imagekit/models/fields/__init__.py +++ b/imagekit/models/fields/__init__.py @@ -45,8 +45,8 @@ class ImageSpecField(SpecHostField): self.set_spec_id(cls, name) # Add the model and field as a source for this spec id - register.sources(self.spec_id, - [ImageFieldSourceGroup(cls, self.source)]) + register.cacheables(self.spec_id, + ImageFieldSourceGroup(cls, self.source)) class ProcessedImageField(models.ImageField, SpecHostField): diff --git a/imagekit/registry.py b/imagekit/registry.py index 8d082cd..93e491f 100644 --- a/imagekit/registry.py +++ b/imagekit/registry.py @@ -1,11 +1,11 @@ from .exceptions import AlreadyRegistered, NotRegistered -from .signals import (before_access, source_created, source_changed, - source_deleted) +from .signals import (before_access, cacheable_created, cacheable_changed, + cacheable_deleted) class GeneratorRegistry(object): """ - An object for registering generators (specs). This registry provides + An object for registering generators. This registry provides a convenient way for a distributable app to define default generators without locking the users of the app into it. @@ -15,7 +15,7 @@ class GeneratorRegistry(object): def register(self, id, generator): if id in self._generators: - raise AlreadyRegistered('The spec or generator with id %s is' + raise AlreadyRegistered('The generator with id %s is' ' already registered' % id) self._generators[id] = generator @@ -24,14 +24,14 @@ class GeneratorRegistry(object): try: del self._generators[id] except KeyError: - raise NotRegistered('The spec or generator with id %s is not' + raise NotRegistered('The generator with id %s is not' ' registered' % id) def get(self, id, **kwargs): try: generator = self._generators[id] except KeyError: - raise NotRegistered('The spec or generator with id %s is not' + raise NotRegistered('The generator with id %s is not' ' registered' % id) if callable(generator): return generator(**kwargs) @@ -42,108 +42,114 @@ class GeneratorRegistry(object): return self._generators.keys() -class SourceGroupRegistry(object): +class CacheableRegistry(object): """ - An object for registering source groups with specs. The two are + An object for registering cacheables with generators. The two are associated with each other via a string id. We do this (as opposed to - associating them directly by, for example, putting a ``source_groups`` - attribute on specs) so that specs can be overridden without losing the - associated sources. That way, a distributable app can define its own - specs without locking the users of the app into it. + associating them directly by, for example, putting a ``cacheables`` + attribute on generators) so that generators can be overridden without + losing the associated cacheables. That way, a distributable app can define + its own generators without locking the users of the app into it. """ _signals = [ - source_created, - source_changed, - source_deleted, + cacheable_created, + cacheable_changed, + cacheable_deleted, ] def __init__(self): - self._source_groups = {} + self._cacheables = {} for signal in self._signals: - signal.connect(self.source_group_receiver) + signal.connect(self.cacheable_receiver) before_access.connect(self.before_access_receiver) - def register(self, spec_id, source_groups): + def register(self, generator_id, cacheables): """ - Associates source groups with a spec id + Associates cacheables with a generator id """ - for source_group in source_groups: - if source_group not in self._source_groups: - self._source_groups[source_group] = set() - self._source_groups[source_group].add(spec_id) + for cacheable in cacheables: + if cacheable not in self._cacheables: + self._cacheables[cacheable] = set() + self._cacheables[cacheable].add(generator_id) - def unregister(self, spec_id, source_groups): + def unregister(self, generator_id, cacheables): """ - Disassociates sources with a spec id + Disassociates cacheables with a generator id """ - for source_group in source_groups: + for cacheable in cacheables: try: - self._source_groups[source_group].remove(spec_id) + self._cacheables[cacheable].remove(generator_id) except KeyError: continue - def get(self, spec_id): - return [source_group for source_group in self._source_groups - if spec_id in self._source_groups[source_group]] + def get(self, generator_id): + return [cacheable for cacheable in self._cacheables + if generator_id in self._cacheables[cacheable]] - def before_access_receiver(self, sender, generator, file, **kwargs): - generator.image_cache_strategy.invoke_callback('before_access', file) + def before_access_receiver(self, sender, generator, cacheable, **kwargs): + generator.image_cache_strategy.invoke_callback('before_access', cacheable) - def source_group_receiver(self, sender, source, signal, info, **kwargs): + def cacheable_receiver(self, sender, cacheable, signal, info, **kwargs): """ - Redirects signals dispatched on sources to the appropriate specs. + Redirects signals dispatched on cacheables + to the appropriate generators. """ - source_group = sender - if source_group not in self._source_groups: + cacheable = sender + if cacheable not in self._cacheables: return - for spec in (generator_registry.get(id, source=source, **info) - for id in self._source_groups[source_group]): + for generator in (generator_registry.get(id, cacheable=cacheable, **info) + for id in self._cacheables[cacheable]): event_name = { - source_created: 'source_created', - source_changed: 'source_changed', - source_deleted: 'source_deleted', + cacheable_created: 'cacheable_created', + cacheable_changed: 'cacheable_changed', + cacheable_deleted: 'cacheable_deleted', } - spec._handle_source_event(event_name, source) + generator._handle_cacheable_event(event_name, cacheable) class Register(object): """ - Register specs and sources. + Register generators and cacheables. """ - def spec(self, id, spec=None): - if spec is None: + def generator(self, id, generator=None): + if generator is None: # Return a decorator def decorator(cls): - self.spec(id, cls) + self.generator(id, cls) return cls return decorator - generator_registry.register(id, spec) + generator_registry.register(id, generator) - def sources(self, spec_id, sources): - source_group_registry.register(spec_id, sources) + # iterable that returns kwargs or callable that returns iterable of kwargs + def cacheables(self, generator_id, cacheables): + if callable(cacheables): + cacheables = cacheables() + cacheable_registry.register(generator_id, cacheables) class Unregister(object): """ - Unregister specs and sources. + Unregister generators and cacheables. """ - def spec(self, id, spec): - generator_registry.unregister(id, spec) + def generator(self, id, generator): + generator_registry.unregister(id, generator) - def sources(self, spec_id, sources): - source_group_registry.unregister(spec_id, sources) + def cacheables(self, generator_id, cacheables): + if callable(cacheables): + cacheables = cacheables() + cacheable_registry.unregister(generator_id, cacheables) generator_registry = GeneratorRegistry() -source_group_registry = SourceGroupRegistry() +cacheable_registry = CacheableRegistry() register = Register() unregister = Unregister() diff --git a/imagekit/signals.py b/imagekit/signals.py index 4c7aefa..97a9d9c 100644 --- a/imagekit/signals.py +++ b/imagekit/signals.py @@ -1,6 +1,6 @@ from django.dispatch import Signal before_access = Signal() -source_created = Signal(providing_args=[]) -source_changed = Signal() -source_deleted = Signal() +cacheable_created = Signal(providing_args=[]) +cacheable_changed = Signal() +cacheable_deleted = Signal() diff --git a/imagekit/specs/__init__.py b/imagekit/specs/__init__.py index 9bfab2c..015d4a6 100644 --- a/imagekit/specs/__init__.py +++ b/imagekit/specs/__init__.py @@ -228,7 +228,7 @@ class SpecHost(object): """ self.spec_id = id - register.spec(id, self._original_spec) + register.generator(id, self._original_spec) def get_spec(self, **kwargs): """ diff --git a/imagekit/specs/sourcegroups.py b/imagekit/specs/sourcegroups.py index b36b211..6bd1eed 100644 --- a/imagekit/specs/sourcegroups.py +++ b/imagekit/specs/sourcegroups.py @@ -1,6 +1,6 @@ from django.db.models.signals import post_init, post_save, post_delete from django.utils.functional import wraps -from ..signals import source_created, source_changed, source_deleted +from ..signals import cacheable_created, cacheable_changed, cacheable_deleted def ik_model_receiver(fn): @@ -58,23 +58,25 @@ class ModelSignalRouter(object): src in self._source_groups if src.model_class is instance.__class__) @ik_model_receiver - def post_save_receiver(self, sender, instance=None, created=False, raw=False, **kwargs): + def post_save_receiver(self, sender, instance=None, created=False, + raw=False, **kwargs): if not raw: 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 created: - self.dispatch_signal(source_created, file, sender, instance, - attname) + self.dispatch_signal(cacheable_created, file, sender, + instance, attname) elif old_hashes[attname] != new_hashes[attname]: - self.dispatch_signal(source_changed, file, sender, instance, - attname) + self.dispatch_signal(cacheable_changed, file, sender, + instance, attname) @ik_model_receiver def post_delete_receiver(self, sender, instance=None, **kwargs): for attname, file in self.get_field_dict(instance).items(): - self.dispatch_signal(source_deleted, file, sender, instance, attname) + self.dispatch_signal(cacheable_deleted, file, sender, instance, + attname) @ik_model_receiver def post_init_receiver(self, sender, instance=None, **kwargs): @@ -119,5 +121,7 @@ class ImageFieldSourceGroup(object): for instance in self.model_class.objects.all(): yield getattr(instance, self.image_field) + def __call__(self): + return self.files() signal_router = ModelSignalRouter() diff --git a/tests/imagespecs.py b/tests/imagespecs.py index 06d2dab..11e87b3 100644 --- a/tests/imagespecs.py +++ b/tests/imagespecs.py @@ -12,5 +12,5 @@ class ResizeTo1PixelSquare(ImageSpec): super(ResizeTo1PixelSquare, self).__init__(**kwargs) -register.spec('testspec', TestSpec) -register.spec('1pxsq', ResizeTo1PixelSquare) +register.generator('testspec', TestSpec) +register.generator('1pxsq', ResizeTo1PixelSquare) From eb9089e0c8966adea727c2bb9460b371360c79b1 Mon Sep 17 00:00:00 2001 From: Eric Eldredge Date: Thu, 24 Jan 2013 00:04:43 -0500 Subject: [PATCH 02/10] Register cacheables as generators instead of items --- .../management/commands/warmimagecache.py | 21 +++++++-------- imagekit/registry.py | 26 ++++++++----------- imagekit/specs/sourcegroups.py | 7 ++--- 3 files changed, 23 insertions(+), 31 deletions(-) diff --git a/imagekit/management/commands/warmimagecache.py b/imagekit/management/commands/warmimagecache.py index 42a005d..dfe5d46 100644 --- a/imagekit/management/commands/warmimagecache.py +++ b/imagekit/management/commands/warmimagecache.py @@ -19,17 +19,16 @@ class Command(BaseCommand): for generator_id in generators: self.stdout.write('Validating generator: %s\n' % generator_id) - for cacheables in cacheable_registry.get(generator_id): - for cacheable in cacheables.files(): - if cacheable: - generator = generator_registry.get(generator_id, cacheable=cacheable) # TODO: HINTS! (Probably based on cacheable, so this will need to be moved into loop below.) - self.stdout.write(' %s\n' % cacheable) - try: - # TODO: Allow other validation actions through command option - GeneratedImageCacheFile(generator).validate() - except Exception, err: - # TODO: How should we handle failures? Don't want to error, but should call it out more than this. - self.stdout.write(' FAILED: %s\n' % err) + for kwargs in cacheable_registry.get(generator_id): + if kwargs: + generator = generator_registry.get(generator_id, **kwargs) # TODO: HINTS! (Probably based on cacheable, so this will need to be moved into loop below.) + self.stdout.write(' %s\n' % generator) + try: + # TODO: Allow other validation actions through command option + GeneratedImageCacheFile(generator).validate() + except Exception, err: + # TODO: How should we handle failures? Don't want to error, but should call it out more than this. + self.stdout.write(' FAILED: %s\n' % err) def compile_patterns(self, generator_ids): return [re.compile('%s$' % '.*'.join(re.escape(part) for part in id.split('*'))) for id in generator_ids] diff --git a/imagekit/registry.py b/imagekit/registry.py index 93e491f..258f252 100644 --- a/imagekit/registry.py +++ b/imagekit/registry.py @@ -70,25 +70,25 @@ class CacheableRegistry(object): Associates cacheables with a generator id """ - for cacheable in cacheables: - if cacheable not in self._cacheables: - self._cacheables[cacheable] = set() - self._cacheables[cacheable].add(generator_id) + if cacheables not in self._cacheables: + self._cacheables[cacheables] = set() + self._cacheables[cacheables].add(generator_id) def unregister(self, generator_id, cacheables): """ Disassociates cacheables with a generator id """ - for cacheable in cacheables: - try: - self._cacheables[cacheable].remove(generator_id) - except KeyError: - continue + try: + self._cacheables[cacheables].remove(generator_id) + except KeyError: + pass def get(self, generator_id): - return [cacheable for cacheable in self._cacheables - if generator_id in self._cacheables[cacheable]] + for k, v in self._cacheables.items(): + if generator_id in v: + for cacheable in k(): + yield cacheable def before_access_receiver(self, sender, generator, cacheable, **kwargs): generator.image_cache_strategy.invoke_callback('before_access', cacheable) @@ -130,8 +130,6 @@ class Register(object): # iterable that returns kwargs or callable that returns iterable of kwargs def cacheables(self, generator_id, cacheables): - if callable(cacheables): - cacheables = cacheables() cacheable_registry.register(generator_id, cacheables) @@ -144,8 +142,6 @@ class Unregister(object): generator_registry.unregister(id, generator) def cacheables(self, generator_id, cacheables): - if callable(cacheables): - cacheables = cacheables() cacheable_registry.unregister(generator_id, cacheables) diff --git a/imagekit/specs/sourcegroups.py b/imagekit/specs/sourcegroups.py index 6bd1eed..04c30c3 100644 --- a/imagekit/specs/sourcegroups.py +++ b/imagekit/specs/sourcegroups.py @@ -117,11 +117,8 @@ class ImageFieldSourceGroup(object): self.image_field = image_field signal_router.add(self) - def files(self): - for instance in self.model_class.objects.all(): - yield getattr(instance, self.image_field) - def __call__(self): - return self.files() + for instance in self.model_class.objects.all(): + yield {'source': getattr(instance, self.image_field)} signal_router = ModelSignalRouter() From a3e9a080d44b07c78dd00afb8b5a6a2ec9cb103c Mon Sep 17 00:00:00 2001 From: Matthew Tretter Date: Mon, 28 Jan 2013 21:07:35 -0500 Subject: [PATCH 03/10] Revert signal names --- imagekit/imagecache/strategies.py | 6 +++--- imagekit/registry.py | 16 ++++++++-------- imagekit/signals.py | 6 +++--- imagekit/specs/sourcegroups.py | 8 ++++---- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/imagekit/imagecache/strategies.py b/imagekit/imagecache/strategies.py index a0b8f76..56efbd7 100644 --- a/imagekit/imagecache/strategies.py +++ b/imagekit/imagecache/strategies.py @@ -20,13 +20,13 @@ class Optimistic(object): """ - def on_cacheable_created(self, file): + def on_source_created(self, file): validate_now(file) - def on_cacheable_deleted(self, file): + def on_source_deleted(self, file): clear_now(file) - def on_cacheable_changed(self, file): + def on_source_changed(self, file): validate_now(file) diff --git a/imagekit/registry.py b/imagekit/registry.py index 258f252..6f48f9c 100644 --- a/imagekit/registry.py +++ b/imagekit/registry.py @@ -1,6 +1,6 @@ from .exceptions import AlreadyRegistered, NotRegistered -from .signals import (before_access, cacheable_created, cacheable_changed, - cacheable_deleted) +from .signals import (before_access, source_created, source_changed, + source_deleted) class GeneratorRegistry(object): @@ -54,9 +54,9 @@ class CacheableRegistry(object): """ _signals = [ - cacheable_created, - cacheable_changed, - cacheable_deleted, + source_created, + source_changed, + source_deleted, ] def __init__(self): @@ -106,9 +106,9 @@ class CacheableRegistry(object): for generator in (generator_registry.get(id, cacheable=cacheable, **info) for id in self._cacheables[cacheable]): event_name = { - cacheable_created: 'cacheable_created', - cacheable_changed: 'cacheable_changed', - cacheable_deleted: 'cacheable_deleted', + source_created: 'source_created', + source_changed: 'source_changed', + source_deleted: 'source_deleted', } generator._handle_cacheable_event(event_name, cacheable) diff --git a/imagekit/signals.py b/imagekit/signals.py index 97a9d9c..4c7aefa 100644 --- a/imagekit/signals.py +++ b/imagekit/signals.py @@ -1,6 +1,6 @@ from django.dispatch import Signal before_access = Signal() -cacheable_created = Signal(providing_args=[]) -cacheable_changed = Signal() -cacheable_deleted = Signal() +source_created = Signal(providing_args=[]) +source_changed = Signal() +source_deleted = Signal() diff --git a/imagekit/specs/sourcegroups.py b/imagekit/specs/sourcegroups.py index 04c30c3..825276e 100644 --- a/imagekit/specs/sourcegroups.py +++ b/imagekit/specs/sourcegroups.py @@ -1,6 +1,6 @@ from django.db.models.signals import post_init, post_save, post_delete from django.utils.functional import wraps -from ..signals import cacheable_created, cacheable_changed, cacheable_deleted +from ..signals import source_created, source_changed, source_deleted def ik_model_receiver(fn): @@ -66,16 +66,16 @@ class ModelSignalRouter(object): new_hashes = self.update_source_hashes(instance) for attname, file in self.get_field_dict(instance).items(): if created: - self.dispatch_signal(cacheable_created, file, sender, + self.dispatch_signal(source_created, file, sender, instance, attname) elif old_hashes[attname] != new_hashes[attname]: - self.dispatch_signal(cacheable_changed, file, sender, + self.dispatch_signal(source_changed, file, sender, instance, attname) @ik_model_receiver def post_delete_receiver(self, sender, instance=None, **kwargs): for attname, file in self.get_field_dict(instance).items(): - self.dispatch_signal(cacheable_deleted, file, sender, instance, + self.dispatch_signal(source_deleted, file, sender, instance, attname) @ik_model_receiver From 5b4456431814b9a892b97547ea3c3c29e17b41fb Mon Sep 17 00:00:00 2001 From: Matthew Tretter Date: Mon, 28 Jan 2013 21:31:38 -0500 Subject: [PATCH 04/10] Add LazyGeneratedImageCacheFile --- imagekit/files.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/imagekit/files.py b/imagekit/files.py index 1dd8976..c025af3 100644 --- a/imagekit/files.py +++ b/imagekit/files.py @@ -2,7 +2,9 @@ from django.conf import settings from django.core.files.base import ContentFile, File from django.core.files.images import ImageFile from django.utils.encoding import smart_str, smart_unicode +from django.utils.functional import LazyObject import os +from .registry import generator_registry from .signals import before_access from .utils import (format_to_mimetype, extension_to_mimetype, get_logger, get_singleton, generate) @@ -158,3 +160,14 @@ class IKContentFile(ContentFile): def __unicode__(self): return smart_unicode(self.file.name or u'') + + +class LazyGeneratedImageCacheFile(LazyObject): + def __init__(self, generator_id, *args, **kwargs): + super(LazyGeneratedImageCacheFile, self).__init__() + + def setup(): + generator = generator_registry.get(generator_id, *args, **kwargs) + self._wrapped = GeneratedImageCacheFile(generator) + + self.__dict__['_setup'] = setup From 3931b552a0924fa34b3333c4e6605d81c9f83204 Mon Sep 17 00:00:00 2001 From: Matthew Tretter Date: Tue, 29 Jan 2013 01:40:00 -0500 Subject: [PATCH 05/10] Separate source groups and cacheables. This allows a sensible specialized interface for source groups, but also for ImageKit to interact with specs using the generalized image generator interface. --- imagekit/files.py | 2 +- imagekit/imagecache/strategies.py | 5 -- imagekit/models/fields/__init__.py | 4 +- imagekit/registry.py | 104 ++++++++++++++++++++--------- imagekit/signals.py | 6 +- imagekit/specs/__init__.py | 6 -- imagekit/specs/sourcegroups.py | 102 +++++++++++++++++++--------- imagekit/utils.py | 7 ++ 8 files changed, 157 insertions(+), 79 deletions(-) diff --git a/imagekit/files.py b/imagekit/files.py index c025af3..150cfa0 100644 --- a/imagekit/files.py +++ b/imagekit/files.py @@ -103,7 +103,7 @@ class GeneratedImageCacheFile(BaseIKFile, ImageFile): super(GeneratedImageCacheFile, self).__init__(storage=storage) def _require_file(self): - before_access.send(sender=self, generator=self.generator, file=self) + before_access.send(sender=self, file=self) return super(GeneratedImageCacheFile, self)._require_file() def clear(self): diff --git a/imagekit/imagecache/strategies.py b/imagekit/imagecache/strategies.py index 56efbd7..4a3d618 100644 --- a/imagekit/imagecache/strategies.py +++ b/imagekit/imagecache/strategies.py @@ -46,11 +46,6 @@ class StrategyWrapper(object): strategy = strategy() self._wrapped = strategy - def invoke_callback(self, name, *args, **kwargs): - func = getattr(self._wrapped, name, None) - if func: - func(*args, **kwargs) - def __unicode__(self): return unicode(self._wrapped) diff --git a/imagekit/models/fields/__init__.py b/imagekit/models/fields/__init__.py index 449faac..2aea3ca 100644 --- a/imagekit/models/fields/__init__.py +++ b/imagekit/models/fields/__init__.py @@ -45,8 +45,8 @@ class ImageSpecField(SpecHostField): self.set_spec_id(cls, name) # Add the model and field as a source for this spec id - register.cacheables(self.spec_id, - ImageFieldSourceGroup(cls, self.source)) + register.source_group(self.spec_id, + ImageFieldSourceGroup(cls, self.source)) class ProcessedImageField(models.ImageField, SpecHostField): diff --git a/imagekit/registry.py b/imagekit/registry.py index 6f48f9c..d74f6b1 100644 --- a/imagekit/registry.py +++ b/imagekit/registry.py @@ -1,6 +1,6 @@ from .exceptions import AlreadyRegistered, NotRegistered -from .signals import (before_access, source_created, source_changed, - source_deleted) +from .signals import before_access, source_created, source_changed, source_deleted +from .utils import call_strategy_method class GeneratorRegistry(object): @@ -12,6 +12,7 @@ class GeneratorRegistry(object): """ def __init__(self): self._generators = {} + before_access.connect(self.before_access_receiver) def register(self, id, generator): if id in self._generators: @@ -41,6 +42,67 @@ class GeneratorRegistry(object): def get_ids(self): return self._generators.keys() + def before_access_receiver(self, sender, file, **kwargs): + generator = file.generator + if generator in self._generators.values(): + # Only invoke the strategy method for registered generators. + call_strategy_method(generator, 'before_access', file=file) + + +class SourceGroupRegistry(object): + """ + The source group registry is responsible for listening to source_* signals + on source groups, and relaying them to the image cache strategies of the + appropriate generators. + + In addition, registering a new source group also registers its cacheables + generator with the cacheable registry. + + """ + _signals = { + source_created: 'on_source_created', + source_changed: 'on_source_changed', + source_deleted: 'on_source_deleted', + } + + def __init__(self): + self._source_groups = {} + for signal in self._signals.keys(): + signal.connect(self.source_group_receiver) + + def register(self, generator_id, source_group): + from .specs.sourcegroups import SourceGroupCacheablesGenerator + generator_ids = self._source_groups.setdefault(source_group, set()) + generator_ids.add(generator_id) + cacheable_registry.register(generator_id, + SourceGroupCacheablesGenerator(source_group, generator_id)) + + def unregister(self, generator_id, source_group): + from .specs.sourcegroups import SourceGroupCacheablesGenerator + generator_ids = self._source_groups.setdefault(source_group, set()) + if generator_id in generator_ids: + generator_ids.remove(generator_id) + cacheable_registry.unregister(generator_id, + SourceGroupCacheablesGenerator(source_group, generator_id)) + + def source_group_receiver(self, sender, source, signal, **kwargs): + """ + Relay source group signals to the appropriate spec strategy. + + """ + source_group = sender + + # Ignore signals from unregistered groups. + if source_group not in self._source_groups: + return + + specs = [generator_registry.get(id, source=source) for id in + self._source_groups[source_group]] + callback_name = self._signals[signal] + + for spec in specs: + call_strategy_method(spec, callback_name, file=source) + class CacheableRegistry(object): """ @@ -53,17 +115,8 @@ class CacheableRegistry(object): """ - _signals = [ - source_created, - source_changed, - source_deleted, - ] - def __init__(self): self._cacheables = {} - for signal in self._signals: - signal.connect(self.cacheable_receiver) - before_access.connect(self.before_access_receiver) def register(self, generator_id, cacheables): """ @@ -90,28 +143,6 @@ class CacheableRegistry(object): for cacheable in k(): yield cacheable - def before_access_receiver(self, sender, generator, cacheable, **kwargs): - generator.image_cache_strategy.invoke_callback('before_access', cacheable) - - def cacheable_receiver(self, sender, cacheable, signal, info, **kwargs): - """ - Redirects signals dispatched on cacheables - to the appropriate generators. - - """ - cacheable = sender - if cacheable not in self._cacheables: - return - - for generator in (generator_registry.get(id, cacheable=cacheable, **info) - for id in self._cacheables[cacheable]): - event_name = { - source_created: 'source_created', - source_changed: 'source_changed', - source_deleted: 'source_deleted', - } - generator._handle_cacheable_event(event_name, cacheable) - class Register(object): """ @@ -132,6 +163,9 @@ class Register(object): def cacheables(self, generator_id, cacheables): cacheable_registry.register(generator_id, cacheables) + def source_group(self, generator_id, source_group): + source_group_registry.register(generator_id, source_group) + class Unregister(object): """ @@ -144,8 +178,12 @@ class Unregister(object): def cacheables(self, generator_id, cacheables): cacheable_registry.unregister(generator_id, cacheables) + def source_group(self, generator_id, source_group): + source_group_registry.unregister(generator_id, source_group) + generator_registry = GeneratorRegistry() cacheable_registry = CacheableRegistry() +source_group_registry = SourceGroupRegistry() register = Register() unregister = Unregister() diff --git a/imagekit/signals.py b/imagekit/signals.py index 4c7aefa..c01e30e 100644 --- a/imagekit/signals.py +++ b/imagekit/signals.py @@ -1,6 +1,10 @@ from django.dispatch import Signal + +# "Cacheables" (cache file) signals before_access = Signal() -source_created = Signal(providing_args=[]) + +# Source group signals +source_created = Signal() source_changed = Signal() source_deleted = Signal() diff --git a/imagekit/specs/__init__.py b/imagekit/specs/__init__.py index 01c7131..7b92845 100644 --- a/imagekit/specs/__init__.py +++ b/imagekit/specs/__init__.py @@ -3,7 +3,6 @@ from django.db.models.fields.files import ImageFieldFile from hashlib import md5 import os import pickle -from ..files import GeneratedImageCacheFile from ..imagecache.backends import get_default_image_cache_backend from ..imagecache.strategies import StrategyWrapper from ..processors import ProcessorPipeline @@ -43,11 +42,6 @@ class BaseImageSpec(object): def generate(self): raise NotImplementedError - # TODO: I don't like this interface. Is there a standard Python one? pubsub? - def _handle_source_event(self, event_name, source): - file = GeneratedImageCacheFile(self) - self.image_cache_strategy.invoke_callback('on_%s' % event_name, file) - class ImageSpec(BaseImageSpec): """ diff --git a/imagekit/specs/sourcegroups.py b/imagekit/specs/sourcegroups.py index 825276e..3c23d64 100644 --- a/imagekit/specs/sourcegroups.py +++ b/imagekit/specs/sourcegroups.py @@ -1,12 +1,26 @@ +""" +Source groups are the means by which image spec sources are identified. They +have two responsibilities: + +1. To dispatch ``source_created``, ``source_changed``, and ``source_deleted`` + signals. (These will be relayed to the corresponding specs' image cache + strategies.) +2. To provide the source files that they represent, via a generator method named + ``files()``. (This is used by the warmimagecache management command for + "pre-caching" image files.) + +""" + from django.db.models.signals import post_init, post_save, post_delete from django.utils.functional import wraps +from ..files import LazyGeneratedImageCacheFile from ..signals import source_created, source_changed, source_deleted def ik_model_receiver(fn): """ A method decorator that filters out signals coming from models that don't - have fields that function as ImageFieldSourceGroup + have fields that function as ImageFieldSourceGroup sources. """ @wraps(fn) @@ -18,8 +32,15 @@ def ik_model_receiver(fn): class ModelSignalRouter(object): """ - Handles signals dispatched by models and relays them to the spec source - groups that represent those models. + Normally, ``ImageFieldSourceGroup`` would be directly responsible for + watching for changes on the model field it represents. However, Django does + not dispatch events for abstract base classes. Therefore, we must listen for + the signals on all models and filter out those that aren't represented by + ``ImageFieldSourceGroup``s. This class encapsulates that functionality. + + Related: + https://github.com/jdriscoll/django-imagekit/issues/126 + https://code.djangoproject.com/ticket/9318 """ @@ -58,25 +79,23 @@ class ModelSignalRouter(object): src in self._source_groups if src.model_class is instance.__class__) @ik_model_receiver - def post_save_receiver(self, sender, instance=None, created=False, - raw=False, **kwargs): + def post_save_receiver(self, sender, instance=None, created=False, raw=False, **kwargs): if not raw: 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 created: - self.dispatch_signal(source_created, file, sender, - instance, attname) + self.dispatch_signal(source_created, file, sender, instance, + attname) elif old_hashes[attname] != new_hashes[attname]: - self.dispatch_signal(source_changed, file, sender, - instance, attname) + self.dispatch_signal(source_changed, file, sender, instance, + attname) @ik_model_receiver def post_delete_receiver(self, sender, instance=None, **kwargs): for attname, file in self.get_field_dict(instance).items(): - self.dispatch_signal(source_deleted, file, sender, instance, - attname) + self.dispatch_signal(source_deleted, file, sender, instance, attname) @ik_model_receiver def post_init_receiver(self, sender, instance=None, **kwargs): @@ -91,34 +110,55 @@ class ModelSignalRouter(object): """ for source_group in self._source_groups: if source_group.model_class is model_class and source_group.image_field == attname: - info = dict( - source_group=source_group, - instance=instance, - field_name=attname, - ) - signal.send(sender=source_group, source=file, info=info) + signal.send(sender=source_group, source=file) class ImageFieldSourceGroup(object): + """ + A source group that repesents a particular field across all instances of a + model. + + """ def __init__(self, model_class, image_field): - """ - Good design would dictate that this instance would be responsible for - watching for changes for the provided field. However, due to a bug in - Django, we can't do that without leaving abstract base models (which - don't trigger signals) in the lurch. So instead, we do all signal - handling through the signal router. - - Related: - https://github.com/jdriscoll/django-imagekit/issues/126 - https://code.djangoproject.com/ticket/9318 - - """ self.model_class = model_class self.image_field = image_field signal_router.add(self) - def __call__(self): + def files(self): + """ + A generator that returns the source files that this source group + represents; in this case, a particular field of every instance of a + particular model. + + """ for instance in self.model_class.objects.all(): - yield {'source': getattr(instance, self.image_field)} + yield getattr(instance, self.image_field) + + +class SourceGroupCacheablesGenerator(object): + """ + A cacheables generator for source groups. The purpose of this class is to + generate cacheables (cache files) from a source group. + + """ + def __init__(self, source_group, generator_id): + self.source_group = source_group + self.generator_id = generator_id + + def __eq__(self, other): + return (isinstance(other, self.__class__) + and self.__dict__ == other.__dict__) + + def __ne__(self, other): + return not self.__eq__(other) + + def __hash__(self): + return hash((self.source_group, self.generator_id)) + + def __call__(self): + for source_file in self.source_group.files(): + yield LazyGeneratedImageCacheFile(self.generator_id, + source=source_file) + signal_router = ModelSignalRouter() diff --git a/imagekit/utils.py b/imagekit/utils.py index 82a2c02..adeb377 100644 --- a/imagekit/utils.py +++ b/imagekit/utils.py @@ -422,3 +422,10 @@ def generate(generator): content = f return File(content) + + +def call_strategy_method(generator, method_name, *args, **kwargs): + strategy = getattr(generator, 'image_cache_strategy', None) + fn = getattr(strategy, method_name, None) + if fn is not None: + fn(*args, **kwargs) From ca4f090e63f7415905db8a71f8ae39ff00bb52a1 Mon Sep 17 00:00:00 2001 From: Matthew Tretter Date: Tue, 29 Jan 2013 01:48:06 -0500 Subject: [PATCH 06/10] Fix source callbacks on strategies --- imagekit/imagecache/strategies.py | 3 ++- imagekit/registry.py | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/imagekit/imagecache/strategies.py b/imagekit/imagecache/strategies.py index 4a3d618..cb7edd4 100644 --- a/imagekit/imagecache/strategies.py +++ b/imagekit/imagecache/strategies.py @@ -1,3 +1,4 @@ +from django.utils.functional import LazyObject from .actions import validate_now, clear_now from ..utils import get_singleton @@ -36,7 +37,7 @@ class DictStrategy(object): setattr(self, k, v) -class StrategyWrapper(object): +class StrategyWrapper(LazyObject): def __init__(self, strategy): if isinstance(strategy, basestring): strategy = get_singleton(strategy, 'image cache strategy') diff --git a/imagekit/registry.py b/imagekit/registry.py index d74f6b1..764ba24 100644 --- a/imagekit/registry.py +++ b/imagekit/registry.py @@ -90,6 +90,7 @@ class SourceGroupRegistry(object): Relay source group signals to the appropriate spec strategy. """ + from .files import GeneratedImageCacheFile source_group = sender # Ignore signals from unregistered groups. @@ -101,7 +102,8 @@ class SourceGroupRegistry(object): callback_name = self._signals[signal] for spec in specs: - call_strategy_method(spec, callback_name, file=source) + file = GeneratedImageCacheFile(spec) + call_strategy_method(spec, callback_name, file=file) class CacheableRegistry(object): From e48817a5ecbe72bb2f6e3b6c6d3d5dd44c9ee2d6 Mon Sep 17 00:00:00 2001 From: Matthew Tretter Date: Tue, 29 Jan 2013 01:53:23 -0500 Subject: [PATCH 07/10] Update warmimagecache to use new cacheable registry --- .../management/commands/warmimagecache.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/imagekit/management/commands/warmimagecache.py b/imagekit/management/commands/warmimagecache.py index dfe5d46..b07c7ec 100644 --- a/imagekit/management/commands/warmimagecache.py +++ b/imagekit/management/commands/warmimagecache.py @@ -1,6 +1,5 @@ from django.core.management.base import BaseCommand import re -from ...files import GeneratedImageCacheFile from ...registry import generator_registry, cacheable_registry @@ -19,16 +18,14 @@ class Command(BaseCommand): for generator_id in generators: self.stdout.write('Validating generator: %s\n' % generator_id) - for kwargs in cacheable_registry.get(generator_id): - if kwargs: - generator = generator_registry.get(generator_id, **kwargs) # TODO: HINTS! (Probably based on cacheable, so this will need to be moved into loop below.) - self.stdout.write(' %s\n' % generator) - try: - # TODO: Allow other validation actions through command option - GeneratedImageCacheFile(generator).validate() - except Exception, err: - # TODO: How should we handle failures? Don't want to error, but should call it out more than this. - self.stdout.write(' FAILED: %s\n' % err) + for cacheable in cacheable_registry.get(generator_id): + self.stdout.write(' %s\n' % cacheable) + try: + # TODO: Allow other validation actions through command option + cacheable.validate() + except Exception, err: + # TODO: How should we handle failures? Don't want to error, but should call it out more than this. + self.stdout.write(' FAILED: %s\n' % err) def compile_patterns(self, generator_ids): return [re.compile('%s$' % '.*'.join(re.escape(part) for part in id.split('*'))) for id in generator_ids] From e0ffb246ae8f253b511e31803d2dcd566a4b9bbb Mon Sep 17 00:00:00 2001 From: Matthew Tretter Date: Tue, 29 Jan 2013 02:17:52 -0500 Subject: [PATCH 08/10] Always use colon as segment separator --- imagekit/models/fields/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/imagekit/models/fields/__init__.py b/imagekit/models/fields/__init__.py index 2aea3ca..35c78da 100644 --- a/imagekit/models/fields/__init__.py +++ b/imagekit/models/fields/__init__.py @@ -11,7 +11,7 @@ class SpecHostField(SpecHost): # Generate a spec_id to register the spec with. The default spec id is # ":_" if not getattr(self, 'spec_id', None): - spec_id = (u'%s:%s_%s' % (cls._meta.app_label, + spec_id = (u'%s:%s:%s' % (cls._meta.app_label, cls._meta.object_name, name)).lower() # Register the spec with the id. This allows specs to be overridden From 54ca5da15d177291585f395ad08c89343e01931c Mon Sep 17 00:00:00 2001 From: Matthew Tretter Date: Tue, 29 Jan 2013 02:18:21 -0500 Subject: [PATCH 09/10] Improve generator id pattern matching This behavior allows users to easy generate images by app, model, or field. --- .../management/commands/warmimagecache.py | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/imagekit/management/commands/warmimagecache.py b/imagekit/management/commands/warmimagecache.py index b07c7ec..9f2a8c8 100644 --- a/imagekit/management/commands/warmimagecache.py +++ b/imagekit/management/commands/warmimagecache.py @@ -4,9 +4,12 @@ from ...registry import generator_registry, cacheable_registry class Command(BaseCommand): - help = ('Warm the image cache for the specified generators' - ' (or all generators if none was provided).' - ' Simple wildcard matching (using asterisks) is supported.') + help = ("""Warm the image cache for the specified generators (or all generators if +none was provided). Simple, fnmatch-like wildcards are allowed, with * +matching all characters within a segment, and ** matching across segments. +(Segments are separated with colons.) So, for example, "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 handle(self, *args, **options): @@ -28,4 +31,16 @@ class Command(BaseCommand): self.stdout.write(' FAILED: %s\n' % err) def compile_patterns(self, generator_ids): - return [re.compile('%s$' % '.*'.join(re.escape(part) for part in id.split('*'))) for id in generator_ids] + return [self.compile_pattern(id) for id in generator_ids] + + def compile_pattern(self, generator_id): + parts = re.split(r'(\*{1,2})', generator_id) + pattern = '' + for part in parts: + if part == '*': + pattern += '[^:]*' + elif part == '**': + pattern += '.*' + else: + pattern += re.escape(part) + return re.compile('^%s(:.*)?$' % pattern) From f0dbe32f7a9fb1781eba214c4eb92b9b9ff6f8f7 Mon Sep 17 00:00:00 2001 From: Matthew Tretter Date: Tue, 29 Jan 2013 02:27:03 -0500 Subject: [PATCH 10/10] Fix pickling error --- imagekit/imagecache/strategies.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/imagekit/imagecache/strategies.py b/imagekit/imagecache/strategies.py index cb7edd4..28068f0 100644 --- a/imagekit/imagecache/strategies.py +++ b/imagekit/imagecache/strategies.py @@ -47,6 +47,12 @@ class StrategyWrapper(LazyObject): strategy = strategy() self._wrapped = strategy + def __getstate__(self): + return {'_wrapped': self._wrapped} + + def __setstate__(self, state): + self._wrapped = state['_wrapped'] + def __unicode__(self): return unicode(self._wrapped)