Opened 8 years ago

Closed 8 years ago

Last modified 5 years ago

#11416 closed (fixed)

Django Admin no longer uses never_cache, breaks with site-wide cache

Reported by: Michael Newman Owned by: nobody
Component: contrib.admin Version: master
Severity: Keywords: admin cache
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

When site cache is enabled using middleware

    'django.middleware.cache.UpdateCacheMiddleware',
    'django.middleware.cache.FetchFromCacheMiddleware',

The admin site is broken because the never_cache decorators have been removed from the views.

Attachments (3)

11416-admin-caching.diff (2.3 KB) - added by Michael Newman 8 years ago.
Patch with a test to ensure there is a max-age on the response from the admin
11416-never-cache-admin-views.diff (7.4 KB) - added by Ramiro Morales 8 years ago.
S slightly modified approach for the patch
11416-admin-caching.2.diff (4.1 KB) - added by Michael Newman 8 years ago.
Slightly modified version of the patch Ramiro uploaded, without some unrelated import and spacing changes and because admin_site.admin_view is used in the wrapper in the ModelAdmin views, there is no need for a decorator change in the ModelAdmin

Download all attachments as: .zip

Change History (10)

comment:1 Changed 8 years ago by James Bennett

Resolution: invalid
Status: newclosed

AdminSite applies the never_cache decorator as part of its internal dispatch to the ModelAdmin class; see around line 500 of contrib/admin.sites.py.

comment:2 Changed 8 years ago by Michael Newman

The model_page function you are referring to is no longer used by the new get_urls function. Am I missing something else?

comment:3 Changed 8 years ago by Michael Newman

Resolution: invalid
Status: closedreopened

I hate to reopen my own ticket, but I am sure that the new urls don't properly receive the cache headers.

Changed 8 years ago by Michael Newman

Attachment: 11416-admin-caching.diff added

Patch with a test to ensure there is a max-age on the response from the admin

Changed 8 years ago by Ramiro Morales

S slightly modified approach for the patch

comment:4 Changed 8 years ago by Ramiro Morales

The attached patch implements something Alex Gaynor suggested on IRC: Moving the addition of never_cache decorator application to the admin_view AdminSite method.

The patchs also:

  • Adds a test for a non-model-specific view like an app index view so both AdminSite and ModelAdmin behaviour is tested.
  • Adds documentation.

Changed 8 years ago by Michael Newman

Attachment: 11416-admin-caching.2.diff added

Slightly modified version of the patch Ramiro uploaded, without some unrelated import and spacing changes and because admin_site.admin_view is used in the wrapper in the ModelAdmin views, there is no need for a decorator change in the ModelAdmin

comment:5 Changed 8 years ago by Michael Newman

Has patch: set
milestone: 1.1

I am going to mark this to 1.1 milestone since it will break some deployments and is unexpected behavior. Also this is functionality that has been downgraded in the admin since 1.0. If someone feels otherwise, please feel free to change.

comment:6 Changed 8 years ago by Russell Keith-Magee

Resolution: fixed
Status: reopenedclosed

(In [11229]) Fixed #11416 -- Restored use of the never_cache decorator on admin views. Thanks to Ramiro Morales and Michael Newmann for their work on the patch.

comment:7 Changed 5 years ago by Jacob

milestone: 1.1

Milestone 1.1 deleted

Note: See TracTickets for help on using tickets.
Back to Top