#33004: Inconsistent / Unexpected handling of assigning unsaved model to Generic
Foreign Key
-------------------------------------+-------------------------------------
Reporter: | Owner: nobody
Finndersen |
Type: | Status: new
Cleanup/optimization |
Component: Database | Version: dev
layer (models, ORM) | Keywords: fk, gfk, generic
Severity: Normal | foreign key, validation
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
https://code.djangoproject.com/ticket/10811 addresses the issue of
assigned an unsaved model to a ForeignKey or OneToOneField (raises error
when save() called), however the same logic / pattern does not apply to
GFKs.
Given:
{{{#!python
class ModelA(models.Model):
name = models.CharField(max_length=20)
class ModelB(models.Model):
gfk_ctype = models.ForeignKey(ContentType, on_delete=models.PROTECT)
gfk_id = models.PositiveIntegerField()
gfk = GenericForeignKey('gfk_ctype', 'gfk_id')
class ModelC(models.Model):
fk = models.ForeignKey(ModelA, on_delete=models.CASCADE)
}}}
Foreign Key Behaviour:
{{{
In [2]: a = ModelA(name='Model A')
In [3]: c = ModelC(fk=a)
In [4]: c.fk
Out[4]: <ModelA: ModelA object (None)>
In [5]: c.save()
---------------------------------------------------------------------------
...
ValueError: save() prohibited to prevent data loss due to unsaved related
object 'fk'.
In [6]: a.save()
(0.016) INSERT INTO "test_app_modela" ("name") VALUES ('Model A');
args=['Model A']
In [7]: c.fk
Out[7]: <ModelA: ModelA object (1)>
In [8]: c.save()
(0.016) INSERT INTO "test_app_modelc" ("fk_id") VALUES (1); args=[1]
}}}
GFK behaviour:
{{{
In [9]: a2 = ModelA(name='Model A2')
In [10]: b = ModelB(gfk=a2)
In [11]: b.gfk
Out[11]: <ModelA: ModelA object (None)>
In [12]: b.save()
(0.000) INSERT INTO "test_app_modelb" ("gfk_ctype_id", "gfk_id") VALUES
(9, NULL); args=[9, None]
---------------------------------------------------------------------------
IntegrityError: NOT NULL constraint failed: test_app_modelb.gfk_id
In [14]: b.gfk.save()
(0.015) INSERT INTO "test_app_modela" ("name") VALUES ('Model A2');
args=['Model A2']
In [15]: b.gfk
(0.000) SELECT "test_app_modela"."id", "test_app_modela"."name" FROM
"test_app_modela" WHERE "test_app_modela"."id" IS NULL LIMIT 21; args=()
None
In [17]: b.gfk_ctype
Out[17]: <ContentType: test_app | model a>
}}}
Two observations:
* No check on b.gfk and b.gfk_id value during save() which could lead to
silent data loss if b.gfk_id is nullable.
* When a2 is saved, accessing b.gfk now does a redundant DB query to try
and find ModelA instance with PK = None, then then returns None value
(effectively un-assigning a2 model), while keeping b.gfk_ctype intact.
This is because the new pk of a2 is different to the existing gfk_id
(pk_val) of the GFK field (None)
What should be done:
* Modify Model.save() or Model._prepare_related_fields_for_save() to also
perform verification check for GFK fields
* Modify GenericForeignKey.__get__() to handle case of pk_val = None
(update fk_field value using PK value of GFK model if present, do not
perform redundant DB query on pk=None, return previously assigned (then
saved) model instead of None)
--
Ticket URL: <https://code.djangoproject.com/ticket/33004>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/053.8deb458a1d22243c93cb974224d2f420%40djangoproject.com.
