Opened 22 months ago

Closed 22 months ago

Last modified 11 months ago

#20442 closed Bug (fixed)

Stale ContentType objects cause a NoneType error in get_for_id

Reported by: vdboor Owned by: vdboor
Component: contrib.contenttypes Version: 1.6
Severity: Normal Keywords: dceu13
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When content types are stale an error occurs when using ContentType.objects.get_for_id():

Traceback (most recent call last):
  ...
  File "/Users/diederik/Sites/webapps/django/django/contrib/contenttypes/tests.py", line 280, in <lambda>
    test = ContentType.objects.get_for_id(ct.pk)
  File "/Users/diederik/Sites/webapps/django/django/contrib/contenttypes/models.py", line 107, in get_for_id
    self._add_to_cache(self.db, ct)
  File "/Users/diederik/Sites/webapps/django/django/contrib/contenttypes/models.py", line 122, in _add_to_cache
    key = (model._meta.app_label, model._meta.model_name)
AttributeError: 'NoneType' object has no attribute '_meta'

The object can still be fetched, but ct.model_class() returns None which causes the _add_to_cache function to operate on a NoneType.

Change History (4)

comment:1 Changed 22 months ago by vdboor

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Status changed from new to assigned
  • Type changed from Uncategorized to Bug

I was thinking about solving this in a few possible ways:

  • Let _add_to_cache use (ct.app_label, ct.model) as key instead of the model object. I think they are always identical, so this would allow caching the stale content type. Nothing changes for calling code, they still need to check whether ct.model_class() returns None as it explicitly does already.
  • Raise a StaleContentTypeError in get_for_id(). This would require updating code everywhere else too.
  • Raise a ContentType.DoesNotExist. This gives consistent output from ContentType.objects.get_for_id() I don't like it as will leak out the inconsistency elsewhere.

My preference goes to the first option.

comment:2 Changed 22 months ago by vdboor

  • Has patch set

When evaluating the solutions with my local projects, it turns out that option nr1 (return the content type) is the best solution.
The get_for_id() returns the ContentType object as promised, and like any other operation (e.g. ContentType.objects.all()), the caller would have to check for ct.model_class() is None.

Pull request: https://github.com/django/django/pull/1130

comment:3 Changed 22 months ago by Andrew Godwin <andrew@…>

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

In 9b22baddef1a6676f82fe7c112894cc8de045334:

Merge pull request #1130 from vdboor/issue_20442_get_for_id

Fixed #20442: NoneType error when fetching a stale ContentType with get_for_id

comment:4 Changed 11 months ago by anonymous

  • Version changed from 1.4 to 1.6
Note: See TracTickets for help on using tickets.
Back to Top