[Django] #18599: GenericForeignKey field can't be set on init of moel
Django
--------------------------------------+--------------------
Reporter: dpantele | Owner: nobody
Type: Bug | Status: new
Component: contrib.contenttypes | Version: 1.4
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+--------------------
If we are trying to set GenericForeignKey to non-saved object during model
initialization, it is not set.
For example:
{{{
class A(models.model):
pass
class B(models.model):
object_id = models.PositiveIntegerField()
content_type = models.ForeignKey(ContentType)
obj = generic.GenericForeignKey()
...
a = A()
b = B(obj=a)
b.obj # None
b.obj.save() # 'NoneType' object has no attribute 'save'
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18599>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
contrib.contenttypes | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by dpantele):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/18599#comment:1>
Django
--------------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.contenttypes | Version: 1.4
Keywords: | Triage Stage: Accepted
Changes (by aaugustin):
* stage: Unreviewed => Accepted
Comment:
The cause is very simple:
[https://github.com/django/django/blob/master/django/contrib/contenttypes/generic.py#L44
GenericForeignKey.instance_pre_init] extracts the content-type and the id
of the related object and then throws the object itself away. If an
unsaved object is passed in the constructor, its id is None, so the
pointer to the object is lost at this point.
Here's what happens with a GFK:
{{{
>>> a = A()
>>> b = B(obj=a)
>>> b.obj
>>> b.content_type
<ContentType: a>
>>> b.object_id
>>> b.save()
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/Users/myk/Documents/dev/django-trunk/django/db/models/base.py",
line 479, in save
force_update=force_update, update_fields=update_fields)
File "/Users/myk/Documents/dev/django-trunk/django/db/models/base.py",
line 574, in save_base
result = manager._insert([self], fields=fields, return_id=update_pk,
using=using, raw=raw)
File "/Users/myk/Documents/dev/django-
trunk/django/db/models/manager.py", line 203, in _insert
return insert_query(self.model, objs, fields, **kwargs)
File "/Users/myk/Documents/dev/django-trunk/django/db/models/query.py",
line 1589, in insert_query
return query.get_compiler(using=using).execute_sql(return_id)
File "/Users/myk/Documents/dev/django-
trunk/django/db/models/sql/compiler.py", line 914, in execute_sql
cursor.execute(sql, params)
File "/Users/myk/Documents/dev/django-trunk/django/db/backends/util.py",
line 42, in execute
return self.cursor.execute(sql, params)
File "/Users/myk/Documents/dev/django-
trunk/django/db/backends/sqlite3/base.py", line 340, in execute
return Database.Cursor.execute(self, query, params)
IntegrityError: test_app_b.object_id may not be NULL
}}}
In comparison here's what happens with a regular FK:
{{{
>>> a = A()
>>> b = B(fk=a)
>>> b.fk
<A: A object>
>>> b.save()
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/Users/myk/Documents/dev/django-trunk/django/db/models/base.py",
line 479, in save
force_update=force_update, update_fields=update_fields)
File "/Users/myk/Documents/dev/django-trunk/django/db/models/base.py",
line 574, in save_base
result = manager._insert([self], fields=fields, return_id=update_pk,
using=using, raw=raw)
File "/Users/myk/Documents/dev/django-
trunk/django/db/models/manager.py", line 203, in _insert
return insert_query(self.model, objs, fields, **kwargs)
File "/Users/myk/Documents/dev/django-trunk/django/db/models/query.py",
line 1589, in insert_query
return query.get_compiler(using=using).execute_sql(return_id)
File "/Users/myk/Documents/dev/django-
trunk/django/db/models/sql/compiler.py", line 914, in execute_sql
cursor.execute(sql, params)
File "/Users/myk/Documents/dev/django-trunk/django/db/backends/util.py",
line 42, in execute
return self.cursor.execute(sql, params)
File "/Users/myk/Documents/dev/django-
trunk/django/db/backends/sqlite3/base.py", line 340, in execute
return Database.Cursor.execute(self, query, params)
IntegrityError: test_app_b.fk_id may not be NULL
}}}
In both cases, saving the B object will fail because the A object doesn't
have an id yet, so there isn't much to gain in "fixing" this.
We should probably just raise an exception in this situation.
--
Ticket URL: <https://code.djangoproject.com/ticket/18599#comment:2>
Django
--------------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.contenttypes | Version: 1.4
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Changes (by dpantele):
* cc: dpantele (added)
Comment:
It is very strange that any object which is passed to the constructor is
thrown away. In FK situation I can do like that:
{{{
>>> a = A()
>>> b = B(fk=a)
>>> b.fk.save()
>>> b.fk_id = b.fk.id
>>> b.save()
}}}
In the GFK situation it is not possible for me to save b object if i don't
pass a. So, I should everywhere pass two references or use an explicit
notation:
{{{
>>> a = A()
>>> b = B()
>>> b.gfk = a
>>> b.gfk.save()
>>> b.gfk = b.gfk
>>> b.save()
}}}
Of course, it should be possible to keep reference to non-saved object.
For example, I need to validate object 'b', and if it is not valid, I
should not save 'a' object at all.
I see very simple way of solving this problem: we should keep cached
object not in object instance, but in the field instance.
--
Ticket URL: <https://code.djangoproject.com/ticket/18599#comment:3>
Django
--------------------------------------+------------------------------------
Reporter: dpantele | Owner: nobody
Type: Bug | Status: new
Component: contrib.contenttypes | Version: 1.4
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by ramiro):
See also #7551.
--
Ticket URL: <https://code.djangoproject.com/ticket/18599#comment:4>
Django
--------------------------------------+------------------------------------
Reporter: dpantele | Owner: nobody
Type: Bug | Status: new
Component: contrib.contenttypes | Version: 1.4
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by bouke):
See also #16508, as this is a common problem of virtual fields
(implementation of GenericForeignKey).
--
Ticket URL: <https://code.djangoproject.com/ticket/18599#comment:5>
Django
--------------------------------------+------------------------------------
Reporter: dpantele | Owner: nobody
Type: Bug | Status: new
Component: contrib.contenttypes | Version: 1.4
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
Comment:
This was fixed in 8a47ba679d2da0dee74671a53ba0cd918b433e34.
[https://github.com/django/django/pull/6599 PR] to add the test.
--
Ticket URL: <https://code.djangoproject.com/ticket/18599#comment:6>
Django
--------------------------------------+------------------------------------
Reporter: dpantele | Owner: nobody
Type: Bug | Status: new
Component: contrib.contenttypes | Version: 1.4
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"31501fb53e0987d0e3ddb1eaddd4ccee1feae407" 31501fb]:
{{{
#!CommitTicketReference repository=""
revision="31501fb53e0987d0e3ddb1eaddd4ccee1feae407"
Refs #18599 -- Added a test for assigning a GenericForeignKey in
Model.__init__().
The issue was fixed by 8a47ba679d2da0dee74671a53ba0cd918b433e34
(refs #16508).
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18599#comment:7>
Django
--------------------------------------+------------------------------------
Reporter: dpantele | Owner: nobody
Type: Bug | Status: closed
Component: contrib.contenttypes | Version: 1.4
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: new => closed
* resolution: => fixed
--
Ticket URL: <https://code.djangoproject.com/ticket/18599#comment:8>