diff --git a/CHANGELOG.md b/CHANGELOG.md index b53a9e3..e1ce0b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +# 0.8.0 (2021-03-19) + +Change default caching headers to `cache-control: max-age=60, stale-while-revalidate=86400, stale-if-error=300`. +Make them individually overrideable. + # 0.7.2 (2021-01-07) ### Added diff --git a/canonicalwebteam/flask_base/app.py b/canonicalwebteam/flask_base/app.py index 205e6d5..fc1597d 100644 --- a/canonicalwebteam/flask_base/app.py +++ b/canonicalwebteam/flask_base/app.py @@ -36,18 +36,91 @@ def set_security_headers(response): def set_cache_control_headers(response): + """ + Default caching rules that should work for most pages + """ + if flask.request.path.startswith("/_status"): + # Our status endpoints need to be uncached + # to report accurate information at all times response.cache_control.no_store = True - response.cache_control.max_age = 0 - elif not response.cache_control and response.status_code == 200: - # response.cache_control does not support stale_while_revalidate so... - response.headers[ - "Cache-Control" - ] = "public, max-age=300, stale-while-revalidate=360" + elif ( - flask.request.path.startswith("/static") and "v" in flask.request.args + response.status_code == 200 + and not response.cache_control.no_store + and not response.cache_control.no_cache + and not response.cache_control.private ): - response.headers["Cache-Control"] = "public, max-age=31536000" + # Normal responses, where the cache-control object hasn't + # been independently modified, should: + + if not response.cache_control.max_age: + # Hard-cache for a minimal amount of time so content can be easily + # refreshed. + # + # The considerations here are as follows: + # - To avoid caching headaches, it's best if when people need to + # refresh a page (e.g. after they've just done a release) they + # can do so by simply refreshing their browser. + # - However, it needs to be long enough to protect our + # applications from excessive requests from all the pages being + # generated (a factor) + # + # 1 minute seems like a good compromise. + response.cache_control.max_age = "60" + + if not response.cache_control._get_cache_value( + "stale-while-revalidate", False, int + ): + # stale-while-revalidate defines a period after the cache has + # expired (max-age) during which users will get sent the stale + # cache, while the cache updates in the background. This mean + # that users will continue to get speedy (potentially stale) + # responses even if the refreshing of the cache takes a while, + # but the content should still be no more than a few seconds + # out of date. + # + # We want this to be pretty long, so users will still get a + # quick response (while triggering a background update) for + # as long as possible after the cache has expired. + # + # An additional day will hopefully be long enough for most cases. + response.cache_control._set_cache_value( + "stale-while-revalidate", "86400", int + ) + + if not response.cache_control._get_cache_value( + "stale-if-error", False, int + ): + # stale-if-error defines a period of time during which a stale + # cache will be served back to the client if the cache observes + # an error code response (>=500) from the backend. When the cache + # receives a request for an erroring page, after serving the stale + # page it will ping off a background request to attempt the + # revalidate the page, as long as it's not currently waiting for a + # response. + # + # We set this value to protect us from transitory errors. The + # trade-off here is that we may also be masking errors from + # ourselves. While we have Sentry and Greylog to potentially alert + # us, we might miss these alerts, which could lead to much more + # serious issues (e.g. bringing down the whole site). The most + # clear manifestation of these error would be simply an error on + # the site. + # + # So we set this to 5 minutes following expiry as a trade-off. + response.cache_control._set_cache_value( + "stale-if-error", "300", int + ) + + # TODO: Disabling this until we've got to the bottom of the + # cache-poisoning issue for static files + # + # elif ( + # flask.request.path.startswith("/static") and + # "v" in flask.request.args + # ): + # response.headers["Cache-Control"] = "public, max-age=31536000" return response diff --git a/setup.py b/setup.py index ab04dac..97dd21b 100755 --- a/setup.py +++ b/setup.py @@ -1,8 +1,10 @@ +#! /usr/bin/env python3 + from setuptools import setup, find_packages setup( name="canonicalwebteam.flask-base", - version="0.7.3", + version="0.8.0", description=( "Flask extension that applies common configurations" "to all of webteam's flask apps." diff --git a/tests/test_app/webapp/app.py b/tests/test_app/webapp/app.py index 720ce3e..cd876b1 100644 --- a/tests/test_app/webapp/app.py +++ b/tests/test_app/webapp/app.py @@ -34,11 +34,42 @@ def soft_redirect(): flask.session["test"] = True return "Found", 302 - @app.route("/cache") - def cache(): + @app.route("/cache/max-age") + def cache_max_age(): response = flask.make_response() + response.cache_control.max_age = 4321 + + return response, 200 + + @app.route("/cache/none") + def cache_empty(): + response = flask.make_response() + response.cache_control.max_age = None + response.cache_control._set_cache_value( + "stale-while-revalidate", None, int + ) + response.cache_control._set_cache_value("stale-if-error", None, int) + + return response, 200 + + @app.route("/cache/stale") + def cache_stale(): + response = flask.make_response() + response.cache_control._set_cache_value( + "stale-while-revalidate", 4321, int + ) + + return response, 200 + + @app.route("/cache/all") + def cache_all(): + response = flask.make_response() + response.cache_control.max_age = 4321 response.cache_control.public = True - response.cache_control.max_age = 1000 + response.cache_control._set_cache_value( + "stale-while-revalidate", 4321, int + ) + response.cache_control._set_cache_value("stale-if-error", 4321, int) return response, 200 diff --git a/tests/test_flask_base.py b/tests/test_flask_base.py index c93f2da..9e5cc29 100644 --- a/tests/test_flask_base.py +++ b/tests/test_flask_base.py @@ -63,10 +63,11 @@ def test_security_headers(self): def test_default_cache_headers(self): with create_test_app().test_client() as client: cached_response = client.get("page") - self.assertEqual( - cached_response.headers.get("Cache-Control"), - "public, max-age=300, stale-while-revalidate=360", - ) + cache = cached_response.headers.get("Cache-Control") + self.assertNotIn("public", cache) + self.assertIn("max-age=60", cache) + self.assertIn("stale-while-revalidate=86400", cache) + self.assertIn("stale-if-error=300", cache) def test_redirects_have_no_cache_headers(self): with create_test_app().test_client() as client: @@ -82,13 +83,41 @@ def test_vary_cookie_when_session(self): cached_response_with_session.headers.get("Vary"), "Cookie" ) - def test_cache_does_not_overide(self): + def test_cache_override(self): + """ + Check each part of the cache-control instruction can be overridden + individually, or nullified + """ + with create_test_app().test_client() as client: - cached_response = client.get("cache") - self.assertEqual( - cached_response.headers.get("Cache-Control"), - "public, max-age=1000", - ) + # all 3 values are overriden, and "public" is added + all_response = client.get("cache/all") + all_cache = all_response.headers.get("Cache-Control") + self.assertIn("public", all_cache) + self.assertIn("max-age=4321", all_cache) + self.assertIn("stale-while-revalidate=4321", all_cache) + self.assertIn("stale-if-error=4321", all_cache) + + # all values are nullified, leading no cache-control header + none_response = client.get("cache/none") + none_cache = none_response.headers.get("Cache-Control") + self.assertIsNone(none_cache) + + # only max-age is overridden, so the "stale" instructions remain + max_age_response = client.get("cache/max-age") + max_age_cache = max_age_response.headers.get("Cache-Control") + self.assertNotIn("public", max_age_cache) + self.assertIn("max-age=4321", max_age_cache) + self.assertIn("stale-while-revalidate=86400", max_age_cache) + self.assertIn("stale-if-error=300", max_age_cache) + + # only "stale-while-revalidate" is overridden + stale_response = client.get("cache/stale") + stale_cache = stale_response.headers.get("Cache-Control") + self.assertNotIn("public", stale_cache) + self.assertIn("max-age=60", stale_cache) + self.assertIn("stale-while-revalidate=4321", stale_cache) + self.assertIn("stale-if-error=300", stale_cache) def test_status_endpoints(self): with create_test_app().test_client() as client: @@ -96,9 +125,7 @@ def test_status_endpoints(self): response = client.get("_status/check") self.assertEqual(response.status_code, 200) self.assertEqual(response.data.decode(), "a-build-id") - self.assertEqual( - response.headers.get("Cache-Control"), "no-store, max-age=0" - ) + self.assertEqual(response.headers.get("Cache-Control"), "no-store") def test_redirects_deleted(self): """