From 948d209adac566d89f44f073fdd77a371c18e269 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Thu, 3 Oct 2013 13:44:10 -0400 Subject: [PATCH] Fixed #21217 -- Avoid connecting `(pre|post)_init` signals to abstract senders. --- django/contrib/contenttypes/generic.py | 8 ++--- django/db/models/fields/files.py | 4 ++- tests/generic_relations/models.py | 30 ++++++++++++---- tests/model_fields/models.py | 49 ++++++++++++++++++++------ tests/model_fields/test_imagefield.py | 6 ++-- 5 files changed, 71 insertions(+), 26 deletions(-) diff --git a/django/contrib/contenttypes/generic.py b/django/contrib/contenttypes/generic.py index b34847b81d..1fdb1f1894 100644 --- a/django/contrib/contenttypes/generic.py +++ b/django/contrib/contenttypes/generic.py @@ -14,7 +14,7 @@ from django.db.models.fields.related import ForeignObject, ForeignObjectRel from django.db.models.related import PathInfo from django.db.models.sql.where import Constraint from django.forms import ModelForm, ALL_FIELDS -from django.forms.models import (BaseModelFormSet, modelformset_factory, save_instance, +from django.forms.models import (BaseModelFormSet, modelformset_factory, modelform_defines_fields) from django.contrib.admin.options import InlineModelAdmin, flatten_fieldsets from django.contrib.contenttypes.models import ContentType @@ -46,10 +46,10 @@ class GenericForeignKey(six.with_metaclass(RenameGenericForeignKeyMethods)): self.cache_attr = "_%s_cache" % name cls._meta.add_virtual_field(self) - # For some reason I don't totally understand, using weakrefs here doesn't work. - signals.pre_init.connect(self.instance_pre_init, sender=cls, weak=False) + # Only run pre-initialization field assignment on non-abstract models + if not cls._meta.abstract: + signals.pre_init.connect(self.instance_pre_init, sender=cls) - # Connect myself as the descriptor for this field setattr(cls, name, self) def instance_pre_init(self, signal, sender, args, kwargs, **_kwargs): diff --git a/django/db/models/fields/files.py b/django/db/models/fields/files.py index 61e3eebf49..557ec6ec8a 100644 --- a/django/db/models/fields/files.py +++ b/django/db/models/fields/files.py @@ -358,7 +358,9 @@ class ImageField(FileField): # Attach update_dimension_fields so that dimension fields declared # after their corresponding image field don't stay cleared by # Model.__init__, see bug #11196. - signals.post_init.connect(self.update_dimension_fields, sender=cls) + # Only run post-initialization dimension update on non-abstract models + if not cls._meta.abstract: + signals.post_init.connect(self.update_dimension_fields, sender=cls) def update_dimension_fields(self, instance, force=False, *args, **kwargs): """ diff --git a/tests/generic_relations/models.py b/tests/generic_relations/models.py index d819a53c0e..79eb59fe47 100644 --- a/tests/generic_relations/models.py +++ b/tests/generic_relations/models.py @@ -32,28 +32,35 @@ class TaggedItem(models.Model): def __str__(self): return self.tag + class ValuableTaggedItem(TaggedItem): value = models.PositiveIntegerField() -@python_2_unicode_compatible -class Comparison(models.Model): - """ - A model that tests having multiple GenericForeignKeys - """ + +class AbstractComparison(models.Model): comparative = models.CharField(max_length=50) content_type1 = models.ForeignKey(ContentType, related_name="comparative1_set") object_id1 = models.PositiveIntegerField() - content_type2 = models.ForeignKey(ContentType, related_name="comparative2_set") + first_obj = generic.GenericForeignKey(ct_field="content_type1", fk_field="object_id1") + + +@python_2_unicode_compatible +class Comparison(AbstractComparison): + """ + A model that tests having multiple GenericForeignKeys. One is defined + through an inherited abstract model and one defined directly on this class. + """ + content_type2 = models.ForeignKey(ContentType, related_name="comparative2_set") object_id2 = models.PositiveIntegerField() - first_obj = generic.GenericForeignKey(ct_field="content_type1", fk_field="object_id1") other_obj = generic.GenericForeignKey(ct_field="content_type2", fk_field="object_id2") def __str__(self): return "%s is %s than %s" % (self.first_obj, self.comparative, self.other_obj) + @python_2_unicode_compatible class Animal(models.Model): common_name = models.CharField(max_length=150) @@ -67,6 +74,7 @@ class Animal(models.Model): def __str__(self): return self.common_name + @python_2_unicode_compatible class Vegetable(models.Model): name = models.CharField(max_length=150) @@ -77,6 +85,7 @@ class Vegetable(models.Model): def __str__(self): return self.name + @python_2_unicode_compatible class Mineral(models.Model): name = models.CharField(max_length=150) @@ -87,18 +96,22 @@ class Mineral(models.Model): def __str__(self): return self.name + class GeckoManager(models.Manager): def get_queryset(self): return super(GeckoManager, self).get_queryset().filter(has_tail=True) + class Gecko(models.Model): has_tail = models.BooleanField(default=False) objects = GeckoManager() + # To test fix for #11263 class Rock(Mineral): tags = generic.GenericRelation(TaggedItem) + class ManualPK(models.Model): id = models.IntegerField(primary_key=True) tags = generic.GenericRelation(TaggedItem) @@ -110,14 +123,17 @@ class ForProxyModelModel(models.Model): obj = generic.GenericForeignKey(for_concrete_model=False) title = models.CharField(max_length=255, null=True) + class ForConcreteModelModel(models.Model): content_type = models.ForeignKey(ContentType) object_id = models.PositiveIntegerField() obj = generic.GenericForeignKey() + class ConcreteRelatedModel(models.Model): bases = generic.GenericRelation(ForProxyModelModel, for_concrete_model=False) + class ProxyRelatedModel(ConcreteRelatedModel): class Meta: proxy = True diff --git a/tests/model_fields/models.py b/tests/model_fields/models.py index a85dfc4f04..89ea77b7ae 100644 --- a/tests/model_fields/models.py +++ b/tests/model_fields/models.py @@ -18,57 +18,69 @@ class Foo(models.Model): a = models.CharField(max_length=10) d = models.DecimalField(max_digits=5, decimal_places=3) + def get_foo(): return Foo.objects.get(id=1) + class Bar(models.Model): b = models.CharField(max_length=10) a = models.ForeignKey(Foo, default=get_foo) + class Whiz(models.Model): CHOICES = ( ('Group 1', ( - (1,'First'), - (2,'Second'), + (1, 'First'), + (2, 'Second'), ) ), ('Group 2', ( - (3,'Third'), - (4,'Fourth'), + (3, 'Third'), + (4, 'Fourth'), ) ), - (0,'Other'), + (0, 'Other'), ) c = models.IntegerField(choices=CHOICES, null=True) + class BigD(models.Model): d = models.DecimalField(max_digits=38, decimal_places=30) + class BigS(models.Model): s = models.SlugField(max_length=255) + class BigInt(models.Model): value = models.BigIntegerField() - null_value = models.BigIntegerField(null = True, blank = True) + null_value = models.BigIntegerField(null=True, blank=True) + class Post(models.Model): title = models.CharField(max_length=100) body = models.TextField() + class NullBooleanModel(models.Model): nbfield = models.NullBooleanField() + class BooleanModel(models.Model): bfield = models.BooleanField(default=None) string = models.CharField(max_length=10, default='abc') + class FksToBooleans(models.Model): """Model wih FKs to models with {Null,}BooleanField's, #15040""" bf = models.ForeignKey(BooleanModel) nbf = models.ForeignKey(NullBooleanModel) + class RenamedField(models.Model): - modelname = models.IntegerField(name="fieldname", choices=((1,'One'),)) + modelname = models.IntegerField(name="fieldname", choices=((1, 'One'),)) + class VerboseNameField(models.Model): id = models.AutoField("verbose pk", primary_key=True) @@ -99,11 +111,13 @@ class VerboseNameField(models.Model): field21 = models.TimeField("verbose field21") field22 = models.URLField("verbose field22") + # This model isn't used in any test, just here to ensure it validates successfully. # See ticket #16570. class DecimalLessThanOne(models.Model): d = models.DecimalField(max_digits=3, decimal_places=3) + class DataModel(models.Model): short_data = models.BinaryField(max_length=10, default=b'\x08') data = models.BinaryField() @@ -111,6 +125,7 @@ class DataModel(models.Model): ############################################################################### # FileField + class Document(models.Model): myfile = models.FileField(upload_to='unused') @@ -126,7 +141,8 @@ if Image: """ def __init__(self, *args, **kwargs): self.was_opened = False - super(TestImageFieldFile, self).__init__(*args,**kwargs) + super(TestImageFieldFile, self).__init__(*args, **kwargs) + def open(self): self.was_opened = True super(TestImageFieldFile, self).open() @@ -146,15 +162,26 @@ if Image: name = models.CharField(max_length=50) mugshot = TestImageField(storage=temp_storage, upload_to='tests') - class PersonWithHeight(models.Model): + class AbsctractPersonWithHeight(models.Model): """ - Model that defines an ImageField with only one dimension field. + Abstract model that defines an ImageField with only one dimension field + to make sure the dimension update is correctly run on concrete subclass + instance post-initialization. """ - name = models.CharField(max_length=50) mugshot = TestImageField(storage=temp_storage, upload_to='tests', height_field='mugshot_height') mugshot_height = models.PositiveSmallIntegerField() + class Meta: + abstract = True + + class PersonWithHeight(AbsctractPersonWithHeight): + """ + Concrete model that subclass an abctract one with only on dimension + field. + """ + name = models.CharField(max_length=50) + class PersonWithHeightAndWidth(models.Model): """ Model that defines height and width fields after the ImageField. diff --git a/tests/model_fields/test_imagefield.py b/tests/model_fields/test_imagefield.py index ce7d33eb32..ec41247248 100644 --- a/tests/model_fields/test_imagefield.py +++ b/tests/model_fields/test_imagefield.py @@ -134,7 +134,7 @@ class ImageFieldTests(ImageFieldTestMixin, TestCase): p = self.PersonModel.objects.get(name="Joan") path = p.mugshot.path shutil.move(path, path + '.moved') - p2 = self.PersonModel.objects.get(name="Joan") + self.PersonModel.objects.get(name="Joan") def test_delete_when_missing(self): """ @@ -412,7 +412,7 @@ class TwoImageFieldTests(ImageFieldTestMixin, TestCase): # was opened. self.assertEqual(p.mugshot.was_opened, False) self.assertEqual(p.headshot.was_opened, False) - self.check_dimensions(p, 4, 8,'mugshot') + self.check_dimensions(p, 4, 8, 'mugshot') self.check_dimensions(p, 8, 4, 'headshot') # After checking dimensions on the image fields, the files will # have been opened. @@ -422,7 +422,7 @@ class TwoImageFieldTests(ImageFieldTestMixin, TestCase): # check dimensions again, the file should not have opened. p.mugshot.was_opened = False p.headshot.was_opened = False - self.check_dimensions(p, 4, 8,'mugshot') + self.check_dimensions(p, 4, 8, 'mugshot') self.check_dimensions(p, 8, 4, 'headshot') self.assertEqual(p.mugshot.was_opened, False) self.assertEqual(p.headshot.was_opened, False)