Fix and improve caching of diff fragments.
Review Request #7118 — Created March 25, 2015 and submitted
Diff fragments were cached server-side, using memcached, but we had no
caching client-side, due to the lack of any caching headers. It turns
out that we could somewhat frequently invalidate these caches as well,
without meaning to.The cache keys and the URLs were making use of the
AJAX_SERIALvalue to
help with invalidation when things change. However, this value wasn't
stable. It originally was computed from the last modified timestamp of
the bundled templates, but in order to invalidate those when extensions
changed, we incorporated an extension sync value with this. That sync
value would be different every time a thread/process starts, or when
extension configuration changed, or when the matching sync key fell out
of cache. When these things happened, diff caches would invalidate.We now have a new variable,
settings.TEMPLATE_SERIAL, which represents
the actual modified timestamp of the templates. All the diff caches have
been updated to factor this in instead (since extensions don't modify
these templates).Along with the new cache key values, caching headers were added for diff
fragment responses. The end result is that you can load a series of
diffs, restart the web server, load the diffs again, and all requests
will consistently come back as Not Modified.
Loaded the diff viewer, and saw it fetch each request. Inspected the
requests and saw ETags assigned.Reloaded and saw 302 Not Modified responses. Verified through code that
the ETags requested and generated matched.Restarted the dev server and reloaded the diff viewer. Again, 302 Not Modified.
Modified a template and restarted the dev server. Saw it re-fetched the diffs.
Unit tests pass.
| Description | From | Last Updated |
|---|---|---|
|
'reviewboard' imported but unused |
 reviewbot
|
|
|
'datetime' imported but unused |
reviewbot
|
|
|
'utc' imported but unused |
reviewbot
|
|
|
'set_last_modified' imported but unused |
reviewbot
|
|
|
'os' imported but unused |
reviewbot
|
|
|
redefinition of unused 'FileDiffMigrationTests' from line 1041 |
reviewbot
|
|
|
'reviewboard' imported but unused |
reviewbot
|
|
|
While I don't every expect the cache key to contain non-ascii characters, this should call .encode('utf-8') just to make sure … |
 david
|
|
|
redefinition of unused 'FileDiffMigrationTests' from line 1039 |
reviewbot
|
- Change Summary:
-
Fixed the Review Bot complaints relevant to this change.
- Commit:
-
2a0c985fdbad0c35cb839d87d82ae70b9acdc98710da309643a27103868832818b399c46415221d3
- Diff:
-
Revision 2 (+57 -15)
-
Tool: Pyflakes Processed Files: reviewboard/diffviewer/renderers.py reviewboard/test.py reviewboard/diffviewer/views.py reviewboard/diffviewer/tests.py reviewboard/__init__.py Ignored Files: reviewboard/static/rb/js/views/tests/diffFragmentQueueViewTests.js reviewboard/static/rb/js/diffviewer/models/diffReviewableModel.js reviewboard/templates/base.html reviewboard/static/rb/js/diffviewer/models/tests/diffReviewableModelTests.js reviewboard/static/rb/js/views/diffFragmentQueueView.js Tool: PEP8 Style Checker Processed Files: reviewboard/diffviewer/renderers.py reviewboard/test.py reviewboard/diffviewer/views.py reviewboard/diffviewer/tests.py reviewboard/__init__.py Ignored Files: reviewboard/static/rb/js/views/tests/diffFragmentQueueViewTests.js reviewboard/static/rb/js/diffviewer/models/diffReviewableModel.js reviewboard/templates/base.html reviewboard/static/rb/js/diffviewer/models/tests/diffReviewableModelTests.js reviewboard/static/rb/js/views/diffFragmentQueueView.js -
-
Tool: Pyflakes Processed Files: reviewboard/diffviewer/renderers.py reviewboard/test.py reviewboard/diffviewer/views.py reviewboard/diffviewer/tests.py reviewboard/__init__.py Ignored Files: reviewboard/static/rb/js/views/tests/diffFragmentQueueViewTests.js reviewboard/static/rb/js/diffviewer/models/diffReviewableModel.js reviewboard/templates/base.html reviewboard/static/rb/js/diffviewer/models/tests/diffReviewableModelTests.js reviewboard/static/rb/js/views/diffFragmentQueueView.js Tool: PEP8 Style Checker Processed Files: reviewboard/diffviewer/renderers.py reviewboard/test.py reviewboard/diffviewer/views.py reviewboard/diffviewer/tests.py reviewboard/__init__.py Ignored Files: reviewboard/static/rb/js/views/tests/diffFragmentQueueViewTests.js reviewboard/static/rb/js/diffviewer/models/diffReviewableModel.js reviewboard/templates/base.html reviewboard/static/rb/js/diffviewer/models/tests/diffReviewableModelTests.js reviewboard/static/rb/js/views/diffFragmentQueueView.js