Skip to content

  • Projects
  • Groups
  • Snippets
  • Help
    • Loading...
    • Help
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
erp5
erp5
  • Project
    • Project
    • Details
    • Activity
    • Releases
    • Cycle Analytics
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Charts
  • Merge Requests 115
    • Merge Requests 115
  • CI / CD
    • CI / CD
    • Pipelines
    • Jobs
    • Schedules
    • Charts
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Members
    • Members
  • Collapse sidebar
  • Activity
  • Graph
  • Charts
  • Jobs
  • Commits
  • nexedi
  • erp5erp5
  • Merge Requests
  • !1247

Open
Opened Aug 21, 2020 by Kazuhiko Shiozaki@kazuhiko
  • Report abuse
Report abuse

WIP: Override cache control header by caching policy manager

This MR is to revert my 5-years-old change, where CachingPolicyManager 'updates' already-computed Cache-Control header, instead of 'overrides' it.

Honestly, I don't remeber the reason why I introduced such change 5 years ago.

(Note that, at the time of writing this, Caching Policy Manger is NOT called for all requests, but for view() and conversion results only. Try grep _setCacheHeaders.)

Here are several cases affected by this MR :

  • For authenticated user access
  1. CookieCrumbler sets Cache-Control: private for ANY authenticated access.
  2. Caching Policy Manager result is max-age=300, public, like by 'public-conversion-result-no-language' policy in generic ERP5.

Without this MR, the result is Cache-Control: private, max-age=300, public but with this MR, the result is Cache-Control: max-age=300, public.

This is the main reason of this MR, so that logged-in user can have benefit of shared server-side cache for DMS conversion result etc.

  • With tricky assignment in form etc.
  1. In form or whatever called in view() processing, set explicitly RESPONSE.setHeader('Cache-Control', 'no-store')
  2. But Caching Policy Manager result is max-age=300, public

Without this MR, the result is Cache-Control: no-store, max-age=300, public but with this MR, the result is Cache-Control: max-age=300, public.

For me, such no-store, public is non-sense and Caching Policy Manager should be always responsible for any 'matching' case. Also, if we want to support such, we can still create a dedicated Caching Policy by checking like 'no-store' in (request.RESPONSE.getHeader('Cache-Control') or '').lower() in its Predicate configuration.

/cc @romain @jerome @cedric.leninivin @vpelletier

Edited Aug 24, 2020 by Kazuhiko Shiozaki

Check out, review, and merge locally

Step 1. Fetch and check out the branch for this merge request

git fetch origin
git checkout -b override_cache_control_header_by_caching_policy_manager origin/override_cache_control_header_by_caching_policy_manager

Step 2. Review the changes locally

Step 3. Merge the branch and fix any conflicts that come up

git fetch origin
git checkout origin/master
git merge --no-ff override_cache_control_header_by_caching_policy_manager

Step 4. Push the result of the merge to GitLab

git push origin master

Note that pushing to GitLab requires write access to this repository.

Tip: You can also checkout merge requests locally by following these guidelines.

  • Discussion 7
  • Commits 4
  • Changes 2
Assignee
Assign to
None
Milestone
None
Assign milestone
Time tracking
0
Labels
None
Assign labels
  • View project labels
Reference: nexedi/erp5!1247
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备14008524号