Code

Opened 3 years ago

Closed 2 years ago

#17256 closed Bug (fixed)

ContentTypeManager.get_by_natural_key method doesn't cache

Reported by: defaultwombat Owned by: nobody
Component: contrib.contenttypes Version: 1.3
Severity: Normal Keywords: cache contenttypes
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

django.contrib.contenttypes.models.py
ContentTypeManager.get_by_natural_key

Code highlighting:

  def get_by_natural_key(self, app_label, model):
      try:
          ct = self.__class__._cache[self.db][(app_label, model)]
      except KeyError:
          ct = self.get(app_label=app_label, model=model)
      return ct

If the content_type is not in the cache yet, the method uses get() without adding the result to the cache.

I cannot see how #16042 could be solved without fixing this.

Attachments (3)

17107.patch (502 bytes) - added by defaultwombat 3 years ago.
patch for svn r17107
ticket-17256-with-tests.diff (2.6 KB) - added by charettes 3 years ago.
Updated patch with tests
master...ticket-17256-contentypemanager-get_by_natural_key-doesnt-cache.diff (4.4 KB) - added by charettes 2 years ago.

Download all attachments as: .zip

Change History (10)

Changed 3 years ago by defaultwombat

patch for svn r17107

comment:1 Changed 3 years ago by lukeplant

  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Changed 3 years ago by charettes

Updated patch with tests

comment:2 Changed 3 years ago by charettes

  • Needs tests unset

Added tests

comment:3 Changed 2 years ago by charettes

Added a new patch which corrects tests introduced for #16042 in r16737 to actually test that calls to get_by_natural_key cache the ct instead of simulating cache using get_for_model.

The diff is generated from github.

comment:4 Changed 2 years ago by claudep

  • Triage Stage changed from Accepted to Ready for checkin

comment:5 Changed 2 years ago by julien

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

In [17502]:

Fixed #17256 -- Ensured that content types get cached when retrieved by natural key. Thanks, defaultwombat and charettes.

comment:6 follow-up: Changed 2 years ago by davide@…

  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Version changed from SVN to 1.3

Any chance for this fix to be backported in 1.3.X?

comment:7 in reply to: ↑ 6 Changed 2 years ago by ramiro

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

Replying to davide@…:

Any chance for this fix to be backported in 1.3.X?

Unfortunately no.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.