Opened 7 years ago

Closed 7 years ago

Last modified 4 years ago

#8309 closed (fixed)

GenericForeignKey fails when used in abstract base models

Reported by: Wonlay Owned by: jacob
Component: Contrib apps Version: master
Severity: Keywords: GenericForeignKey, model inheritance
Cc: carl@…, wonlay@…, albrecht.andi@…, gonz@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

The following code will reproduce this bug:

# aaa/models.py
from django.contrib.contenttypes.models import ContentType
from django.contrib.contenttypes import generic

class MyModel(models.Model):
	field = models.IntegerField()

class TaggedItemBase(models.Model):
	class Meta:
		abstract = True

	content_type = models.ForeignKey(ContentType)
	object_id = models.PositiveIntegerField()

	content_object = generic.GenericForeignKey()

class TaggedItem(TaggedItemBase):
	"""A tag on an item."""
	tag = models.SlugField()

	class Meta:
		ordering = ["tag"]

	def __unicode__(self):
		return self.tag


# From command shell
>>> from aaa.models import MyModel, TaggedItem
>>> m = MyModel(field=1)
>>> m.save()
>>> t = TaggedItem(content_object=m, tag="my")
Traceback (most recent call last):
  File "<console>", line 1, in ?
  File "/usr/lib/python2.4/site-packages/django/db/models/base.py", line 240, in __init__
    raise TypeError, "'%s' is an invalid keyword argument for this function" % kwargs.keys()[0]
TypeError: 'content_object' is an invalid keyword argument for this function

If we do not use model inheritance, the script works fine.

Attachments (1)

genericforeignkey_fail_in_abstract_models_r8507.diff (4.9 KB) - added by msaelices 7 years ago.
Patch that fixes problem (with tests)

Download all attachments as: .zip

Change History (17)

comment:1 Changed 7 years ago by mtredinnick

  • milestone changed from 1.0 beta to 1.0
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 7 years ago by carljm

  • Cc carl@… added

comment:3 Changed 7 years ago by Wonlay

  • Cc wonlay@… added

It may caused by:

GenericForeignKey.contribute_to_class is called on TaggedItemBase(not on TaggedItem), and register a signal pre_init.

When initialize the subclass TaggedItem, the signal does not fire.

comment:4 Changed 7 years ago by Wonlay

I found that 'content_object' is not in TaggedItem.__dict__, missing due to inheritance?

comment:5 Changed 7 years ago by aalbrecht

  • Cc albrecht.andi@… added

comment:6 Changed 7 years ago by msaelices

  • Owner changed from nobody to msaelices

comment:7 Changed 7 years ago by msaelices

  • Status changed from new to assigned

comment:8 Changed 7 years ago by msaelices

Real problem is in this fragment of django.db.base.ModelBase class:

                # The abstract base class case.
                names = set([f.name for f in new_class._meta.local_fields + new_class._meta.many_to_many])
                for field in base._meta.local_fields + base._meta.local_many_to_many:
                    if field.name in names:
                        raise FieldError('Local field %r in class %r clashes with field of similar name from abstract base class %r'
                                % (field.name, name, base.__name__))
                    new_class.add_to_class(field.name, copy.deepcopy(field))

The problem is django.contrib.generic.GenericForeignKey.contribute_to_class doesnt call to cls._meta.add_field(self), that insert field into base._meta.local_fields.

But the thing is GenericForeignKey is not a real field... is a virtual field. I'll try take a workaround that also implements a new feature, for adding virtual fields thats follow inheritance.

Changed 7 years ago by msaelices

Patch that fixes problem (with tests)

comment:9 Changed 7 years ago by msaelices

  • Has patch set

comment:10 Changed 7 years ago by anonymous

msaelices's patch works fine for me?
Is it time to check in?

comment:11 Changed 7 years ago by msaelices

I'm not a core dev, I think patch works and of course pass all model tests. If a core developer think patch is good, he will check in as soon as possible (it's scheduled for 1.0)

comment:12 Changed 7 years ago by anonymous

  • Cc gonz@… added

comment:13 Changed 7 years ago by jacob

  • Owner changed from msaelices to jacob
  • Status changed from assigned to new

comment:14 Changed 7 years ago by jacob

  • Status changed from new to assigned

comment:15 Changed 7 years ago by jacob

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [8855]) Fixed #8309: subclasses now inherit GenericForeignKey correctly. There's also now an internal API so that other "virtual fields" like GFK can be inherited as well. Thanks, msaelices.

comment:16 Changed 4 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

Note: See TracTickets for help on using tickets.
Back to Top