Opened 6 years ago

Closed 6 years ago

Last modified 3 years ago

#11416 closed (fixed)

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

Reported by: Mnewman 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 Mnewman 6 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 6 years ago.
S slightly modified approach for the patch
11416-admin-caching.2.diff (4.1 KB) - added by Mnewman 6 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 6 years ago by ubernostrum

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to closed

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 6 years ago by Mnewman

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 6 years ago by Mnewman

  • Resolution invalid deleted
  • Status changed from closed to reopened

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

Changed 6 years ago by Mnewman

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

Changed 6 years ago by ramiro

S slightly modified approach for the patch

comment:4 Changed 6 years ago by ramiro

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 6 years ago by Mnewman

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 6 years ago by Mnewman

  • Has patch set
  • milestone set to 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 6 years ago by russellm

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

(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 3 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

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