From db6cfcb6ce7e6465b60d6ca5de6c703ae50892a9 Mon Sep 17 00:00:00 2001 From: Matthew Tretter Date: Sat, 13 Jul 2013 16:38:49 -0400 Subject: [PATCH 1/3] Add (failing) test for #234 --- tests/test_serialization.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_serialization.py b/tests/test_serialization.py index cd7b6d7..85ab157 100644 --- a/tests/test_serialization.py +++ b/tests/test_serialization.py @@ -11,3 +11,15 @@ def test_imagespecfield(): instance = create_photo('pickletest2.jpg') thumbnail = pickleback(instance.thumbnail) thumbnail.generate() + + +def test_circular_ref(): + """ + A model instance with a spec field in its dict shouldn't raise a KeyError. + + This corresponds to #234 + + """ + instance = create_photo('pickletest3.jpg') + instance.thumbnail # Cause thumbnail to be added to instance's __dict__ + pickleback(instance) From 3c04cb852ffaef42255beeb5eec7749bb50d03f7 Mon Sep 17 00:00:00 2001 From: Matthew Tretter Date: Sat, 13 Jul 2013 17:12:55 -0400 Subject: [PATCH 2/3] Don't require source in __setstate__; fixes #234 --- imagekit/specs/__init__.py | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/imagekit/specs/__init__.py b/imagekit/specs/__init__.py index aaa40a6..18f4958 100644 --- a/imagekit/specs/__init__.py +++ b/imagekit/specs/__init__.py @@ -93,25 +93,41 @@ class ImageSpec(BaseImageSpec): fn = get_by_qname(settings.IMAGEKIT_SPEC_CACHEFILE_NAMER, 'namer') return fn(self) + @property + def source(self): + src = getattr(self, '_source', None) + if not src: + field_data = getattr(self, '_field_data', None) + if field_data: + src = self._source = getattr(field_data['instance'], field_data['attname']) + del self._field_data + return src + + @source.setter + def source(self, value): + self._source = value + def __getstate__(self): state = self.__dict__ # Unpickled ImageFieldFiles won't work (they're missing a storage # object). Since they're such a common use case, we special case them. + # Unfortunately, this also requires us to add the source getter to + # lazily retrieve the source on the reconstructed object; simply trying + # to look up the source in ``__setstate__`` would require us to get the + # model instance but, if ``__setstate__`` was called as part of + # deserializing that model, the model wouldn't be fully reconstructed + # yet, preventing us from accessing the source field. + # (This is issue #234.) if isinstance(self.source, ImageFieldFile): field = getattr(self.source, 'field') state['_field_data'] = { 'instance': getattr(self.source, 'instance', None), 'attname': getattr(field, 'name', None), } + state.pop('_source', None) return state - def __setstate__(self, state): - field_data = state.pop('_field_data', None) - self.__dict__ = state - if field_data: - self.source = getattr(field_data['instance'], field_data['attname']) - def get_hash(self): return hashers.pickle([ self.source.name, From 14939faef613bd42ff2550deea37a894f38647d1 Mon Sep 17 00:00:00 2001 From: Matthew Tretter Date: Sat, 13 Jul 2013 17:13:58 -0400 Subject: [PATCH 3/3] Don't mutate __dict__ --- imagekit/specs/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/imagekit/specs/__init__.py b/imagekit/specs/__init__.py index 18f4958..bc33124 100644 --- a/imagekit/specs/__init__.py +++ b/imagekit/specs/__init__.py @@ -1,3 +1,4 @@ +from copy import copy from django.conf import settings from django.db.models.fields.files import ImageFieldFile from ..cachefiles.backends import get_default_cachefile_backend @@ -108,7 +109,7 @@ class ImageSpec(BaseImageSpec): self._source = value def __getstate__(self): - state = self.__dict__ + state = copy(self.__dict__) # Unpickled ImageFieldFiles won't work (they're missing a storage # object). Since they're such a common use case, we special case them.