Opened 12 years ago

Closed 8 years ago

#18599 closed Bug (fixed)

GenericForeignKey field can't be set on init of model

Reported by: dpantele Owned by: nobody
Component: contrib.contenttypes Version: 1.4
Severity: Normal Keywords:
Cc: dpantele Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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'

Attachments (1)

18599-test.patch (614 bytes ) - added by Bouke Haarsma 11 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 by dpantele, 12 years ago

Summary: GenericForeignKey field can't be set on init of moelGenericForeignKey field can't be set on init of model

comment:2 by Aymeric Augustin, 12 years ago

Triage Stage: UnreviewedAccepted

The cause is very simple: 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.

comment:3 by dpantele, 12 years ago

Cc: dpantele added

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.

comment:4 by Ramiro Morales, 11 years ago

See also #7551.

by Bouke Haarsma, 11 years ago

Attachment: 18599-test.patch added

comment:5 by Bouke Haarsma, 11 years ago

See also #16508, as this is a common problem of virtual fields (implementation of GenericForeignKey).

comment:6 by Tim Graham, 8 years ago

Has patch: set

This was fixed in 8a47ba679d2da0dee74671a53ba0cd918b433e34. PR to add the test.

comment:7 by Tim Graham <timograham@…>, 8 years ago

In 31501fb:

Refs #18599 -- Added a test for assigning a GenericForeignKey in Model.init().

The issue was fixed by 8a47ba679d2da0dee74671a53ba0cd918b433e34
(refs #16508).

comment:8 by Tim Graham, 8 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top