[Django] #28613: GenericForeignKey silently returns None when referring to non-existent pk.

8 views
Skip to first unread message

Django

unread,
Sep 19, 2017, 2:00:40 AM9/19/17
to django-...@googlegroups.com
#28613: GenericForeignKey silently returns None when referring to non-existent pk.
-------------------------------------+-------------------------------------
Reporter: Sjoerd | Owner: nobody
Job Postmus |
Type: | Status: new
Uncategorized |
Component: | Version: master
contrib.contenttypes |
Severity: Normal | Keywords: genericforeignkey
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
We have a model as follows.

{{{
class RemoteIdentifierToLocal(models.Model):
remote_id = models.UUIDField(primary_key=True)

local_object_content_type = models.ForeignKey(ContentType)
local_object_object_id = models.PositiveIntegerField()
local_object = GenericForeignKey(ct_field='local_object_content_type',
fk_field='local_object_object_id')
}}}

which is used as follows:

{{{
if guid is not None:
local_object =
RemoteIdentifierToLocal.objects.get(remote_id=guid).local_object
}}}

To my surprise, we found a case where `guid` was not-None, and
`local_object` was None.

This was traced back to the implementation of `GenericForeignKey`
containing a try/except-pass:

https://github.com/django/django/blob/master/django/contrib/contenttypes/fields.py#L241
(now)
https://github.com/django/django/commit/bca5327b21eb2e3ee18292cbe532d6d0071201d8
#diff-cc83843e623c1ab07a24317073330058R62 (initial commit, 11 years ago)

Now, apparently this has been this way for 11 years already, so must be
the "correct" behaviour. However I have not found any documentation of
this fact, and would (myself) expect the attribute-access to *do* raise an
exception or warning of some sort.

--
Ticket URL: <https://code.djangoproject.com/ticket/28613>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Sep 25, 2017, 1:37:20 PM9/25/17
to django-...@googlegroups.com
#28613: Document that GenericForeignKey returns None when referring to nonexistent
pk
--------------------------------------+------------------------------------
Reporter: Sjoerd Job Postmus | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: genericforeignkey | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Tim Graham):

* component: contrib.contenttypes => Documentation
* stage: Unreviewed => Accepted
* type: Uncategorized => Cleanup/optimization


Comment:

There's
[https://github.com/django/django/blob/6da140724dba546d2f3aced1308e617747b0385c/tests/generic_relations/tests.py#L398-L401
one assertion] in Django's test suite that fails with the exception
catching removed. I doubt it's appropriate to raise a warning for properly
working code (assuming the test represents a valid use case -- I didn't
study it in much detail). Accepting at least as a documentation
enhancement.

--
Ticket URL: <https://code.djangoproject.com/ticket/28613#comment:1>

Django

unread,
Sep 30, 2017, 10:21:31 AM9/30/17
to django-...@googlegroups.com
#28613: Document that GenericForeignKey returns None when referring to nonexistent
pk
-------------------------------------+-------------------------------------
Reporter: Sjoerd Job Postmus | Owner: Harshit
Type: | Jain
Cleanup/optimization | Status: assigned

Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: genericforeignkey | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Harshit Jain):

* owner: nobody => Harshit Jain
* status: new => assigned


Comment:

I would like to document this. Can you tell me where do I need to add
this?

--
Ticket URL: <https://code.djangoproject.com/ticket/28613#comment:2>

Django

unread,
Oct 2, 2017, 7:03:26 AM10/2/17
to django-...@googlegroups.com
#28613: Document that GenericForeignKey returns None when referring to nonexistent
pk
-------------------------------------+-------------------------------------
Reporter: Sjoerd Job Postmus | Owner: Harshit
Type: | Jain
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: genericforeignkey | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Harshit Jain):

* has_patch: 0 => 1


Comment:

The Documentation has been added. Here is the link to the pull request -
[https://github.com/django/django/pull/9181].

--
Ticket URL: <https://code.djangoproject.com/ticket/28613#comment:3>

Django

unread,
Oct 2, 2017, 9:33:28 AM10/2/17
to django-...@googlegroups.com
#28613: Document that GenericForeignKey returns None when referring to nonexistent
pk
-------------------------------------+-------------------------------------
Reporter: Sjoerd Job Postmus | Owner: Harshit
Type: | Jain
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: genericforeignkey | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

* needs_better_patch: 0 => 1


--
Ticket URL: <https://code.djangoproject.com/ticket/28613#comment:4>

Django

unread,
Oct 20, 2017, 3:33:20 PM10/20/17
to django-...@googlegroups.com
#28613: Document that GenericForeignKey returns None when referring to nonexistent
pk
-------------------------------------+-------------------------------------
Reporter: Sjoerd Job Postmus | Owner: Harshit
Type: | Jain
Cleanup/optimization | Status: closed
Component: Documentation | Version: master
Severity: Normal | Resolution: fixed

Keywords: genericforeignkey | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"1a82fc245eb8ac4b131ec02a6ac3e112deb8d5a6" 1a82fc2]:
{{{
#!CommitTicketReference repository=""
revision="1a82fc245eb8ac4b131ec02a6ac3e112deb8d5a6"
Fixed #28613 -- Doc'd the return value for GenericForeignKey when the
related object is deleted.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/28613#comment:6>

Django

unread,
Oct 20, 2017, 3:33:20 PM10/20/17
to django-...@googlegroups.com
#28613: Document that GenericForeignKey returns None when referring to nonexistent
pk
-------------------------------------+-------------------------------------
Reporter: Sjoerd Job Postmus | Owner: Harshit
Type: | Jain
Cleanup/optimization | Status: closed
Component: Documentation | Version: master
Severity: Normal | Resolution: fixed
Keywords: genericforeignkey | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham <timograham@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"f9a01d40d5610276cad6b6dc08a27f3030030e2d" f9a01d40]:
{{{
#!CommitTicketReference repository=""
revision="f9a01d40d5610276cad6b6dc08a27f3030030e2d"
[2.0.x] Fixed #28613 -- Doc'd the return value for GenericForeignKey when


the related object is deleted.

Backport of 1a82fc245eb8ac4b131ec02a6ac3e112deb8d5a6 from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/28613#comment:5>

Reply all
Reply to author
Forward
0 new messages