From 2f0cc996104a3cf5a82a243d28073191d1cab41c Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 10 Feb 2016 15:47:00 +0000 Subject: [PATCH 1/2] Make URLs for assets cache-proof MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit https://www.pivotaltracker.com/story/show/113448149 This commit adds a query string to assets URLs which is generated from a hash of the file contents. When asset files are changed they will now be served from a different URL, which means they wont be loaded from browser cache. This is similar to how GOV.UK template adds its version number as a querystring parameter for its assets. This is mostly copied from Digital Marketplace utils: https://github.com/alphagov/digitalmarketplace-utils/pull/102 They have it in a shared codebase, we only have one frontend app so don’t need to do that. Usage in a template: ``` jinja {{ asset_fingerprinter.get_url('stylesheets/application.css') }} ``` Output: ``` static/stylesheets/application.css?418e6f4a6cdf1142e45c072ed3e1c90a ``` --- app/__init__.py | 5 +- app/asset_fingerprinter.py | 46 +++++++++++ app/templates/admin_template.html | 10 +-- tests/app/main/test_asset_fingerprinter.py | 95 ++++++++++++++++++++++ 4 files changed, 150 insertions(+), 6 deletions(-) create mode 100644 app/asset_fingerprinter.py create mode 100644 tests/app/main/test_asset_fingerprinter.py diff --git a/app/__init__.py b/app/__init__.py index e5216b4ad..e85140659 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -18,6 +18,7 @@ from app.notify_client.user_api_client import UserApiClient from app.notify_client.job_api_client import JobApiClient from app.notify_client.status_api_client import StatusApiClient from app.its_dangerous_session import ItsdangerousSessionInterface +from app.asset_fingerprinter import AssetFingerprinter import app.proxy_fix from config import configs from utils import logging @@ -30,6 +31,7 @@ user_api_client = UserApiClient() api_key_api_client = ApiKeyApiClient() job_api_client = JobApiClient() status_api_client = StatusApiClient() +asset_fingerprinter = AssetFingerprinter() def create_app(config_name, config_overrides=None): @@ -106,7 +108,8 @@ def init_app(app, config_overrides): def inject_global_template_variables(): return { 'asset_path': '/static/', - 'header_colour': app.config['HEADER_COLOUR'] + 'header_colour': app.config['HEADER_COLOUR'], + 'asset_url': asset_fingerprinter.get_url } diff --git a/app/asset_fingerprinter.py b/app/asset_fingerprinter.py new file mode 100644 index 000000000..ed1ff9fcb --- /dev/null +++ b/app/asset_fingerprinter.py @@ -0,0 +1,46 @@ +import hashlib +import codecs + + +class AssetFingerprinter(object): + """ + Get a unique hash for an asset file, so that it doesn't stay cached + when it changes + + Usage: + + in the application + template_data.asset_fingerprinter = AssetFingerprinter() + + where template data is how you pass variables to every template. + + in template.html: + {{ asset_fingerprinter.get_url('stylesheets/application.css') }} + + * 'app/static' is assumed to be the root for all asset files + """ + + def __init__(self, asset_root='/static/', filesystem_path='app/static/'): + self._cache = {} + self._asset_root = asset_root + self._filesystem_path = filesystem_path + + def get_url(self, asset_path): + if asset_path not in self._cache: + self._cache[asset_path] = ( + self._asset_root + + asset_path + + '?' + + self.get_asset_fingerprint(self._filesystem_path + asset_path) + ) + return self._cache[asset_path] + + def get_asset_fingerprint(self, asset_file_path): + return hashlib.md5( + self.get_asset_file_contents(asset_file_path).encode('utf-8') + ).hexdigest() + + def get_asset_file_contents(self, asset_file_path): + with codecs.open(asset_file_path, encoding='utf-8') as asset_file: + contents = asset_file.read() + return contents diff --git a/app/templates/admin_template.html b/app/templates/admin_template.html index 9624ca741..82be0d2d0 100644 --- a/app/templates/admin_template.html +++ b/app/templates/admin_template.html @@ -3,19 +3,19 @@ {% block head %} - + {% endblock %} @@ -69,5 +69,5 @@ {% endblock %} {% block body_end %} - + {% endblock %} diff --git a/tests/app/main/test_asset_fingerprinter.py b/tests/app/main/test_asset_fingerprinter.py new file mode 100644 index 000000000..acdaa6fef --- /dev/null +++ b/tests/app/main/test_asset_fingerprinter.py @@ -0,0 +1,95 @@ +# coding=utf-8 +import os + +from unittest import mock + +from app.asset_fingerprinter import AssetFingerprinter + + +@mock.patch.object(AssetFingerprinter, 'get_asset_file_contents') +class TestAssetFingerprint(object): + def test_url_format(self, get_file_content_mock): + get_file_content_mock.return_value = """ + body { + font-family: nta; + } + """ + asset_fingerprinter = AssetFingerprinter( + asset_root='/suppliers/static/' + ) + assert ( + asset_fingerprinter.get_url('application.css') == + '/suppliers/static/application.css?418e6f4a6cdf1142e45c072ed3e1c90a' # noqa + ) + assert ( + asset_fingerprinter.get_url('application-ie6.css') == + '/suppliers/static/application-ie6.css?418e6f4a6cdf1142e45c072ed3e1c90a' # noqa + ) + + def test_building_file_path(self, get_file_content_mock): + get_file_content_mock.return_value = """ + document.write('Hello world!'); + """ + fingerprinter = AssetFingerprinter() + fingerprinter.get_url('javascripts/application.js') + fingerprinter.get_asset_file_contents.assert_called_with( + 'app/static/javascripts/application.js' + ) + + def test_hashes_are_consistent(self, get_file_content_mock): + get_file_content_mock.return_value = """ + body { + font-family: nta; + } + """ + asset_fingerprinter = AssetFingerprinter() + assert ( + asset_fingerprinter.get_asset_fingerprint('application.css') == + asset_fingerprinter.get_asset_fingerprint('same_contents.css') + ) + + def test_hashes_are_different_for_different_files( + self, get_file_content_mock + ): + asset_fingerprinter = AssetFingerprinter() + get_file_content_mock.return_value = """ + body { + font-family: nta; + } + """ + css_hash = asset_fingerprinter.get_asset_fingerprint('application.css') + get_file_content_mock.return_value = """ + document.write('Hello world!'); + """ + js_hash = asset_fingerprinter.get_asset_fingerprint('application.js') + assert ( + js_hash != css_hash + ) + + def test_hash_gets_cached(self, get_file_content_mock): + get_file_content_mock.return_value = """ + body { + font-family: nta; + } + """ + fingerprinter = AssetFingerprinter() + assert ( + fingerprinter.get_url('application.css') == + '/static/application.css?418e6f4a6cdf1142e45c072ed3e1c90a' + ) + fingerprinter._cache[ + 'application.css' + ] = 'a1a1a1' + assert ( + fingerprinter.get_url('application.css') == + 'a1a1a1' + ) + fingerprinter.get_asset_file_contents.assert_called_once_with( + 'app/static/application.css' + ) + + +class TestAssetFingerprintWithUnicode(object): + def test_can_read_self(self): + string_with_unicode_character = 'Ralph’s apostrophe' + AssetFingerprinter(filesystem_path='tests/app/main/').get_url('test_asset_fingerprinter.py') From fc09750662bf72c707db4cf2ca748e620b47d2af Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 10 Feb 2016 16:07:10 +0000 Subject: [PATCH 2/2] Cache static files for a year We only want static files to not come from the browser cache when they have changed. The best way to do this is by cache busting the URLs. Otherwise, we want static files to be cached for a long time. This commit sets the `Expires` HTTP header to 1 year in the future. Previously it was set to 12 hours, the default. From the Flask docs: > Default cache control max age to use with send_static_file() (the default > static file handler) and send_file(), in seconds. Override this value on a > per-file basis using the get_send_file_max_age() hook on Flask or Blueprint, > respectively. Defaults to 43200 (12 hours). --- config.py | 1 + 1 file changed, 1 insertion(+) diff --git a/config.py b/config.py index 55ef41723..bc1ae87db 100644 --- a/config.py +++ b/config.py @@ -5,6 +5,7 @@ class Config(object): DEBUG = False ASSETS_DEBUG = False cache = False + SEND_FILE_MAX_AGE_DEFAULT = 365 * 24 * 60 * 60 # 1 year manifest = True NOTIFY_LOG_LEVEL = 'DEBUG'