Use cache-busted URLs for image assets

Replace hard-coded '/asset' URL paths in templates with cache-busted URLs generated from the site's static asset manifest. * Add `asset_url` helper for retrieving the cache-busted URL for an asset with a given path and use it in templates. * Fix missing entries in asset manifest by including subdirectories (eg. 'images/icons/*.svg') and all extensions (eg. sourcemaps) See #4117
hypothesis · Nov 28, 2016 · 91d3ec7 · 91d3ec7
1 parent 9b43e29
commit 91d3ec7
Show file tree

Hide file tree

Showing 7 changed files with 25 additions and 10 deletions.
diff --git a/gulpfile.js b/gulpfile.js
@@ -208,7 +208,7 @@ gulp.task('watch-images', function () {
   gulp.watch(imageFiles, ['build-images']);
 });
 
-var MANIFEST_SOURCE_FILES = 'build/@(fonts|images|scripts|styles)/*.@(js|css|woff|jpg|png|svg)';
+var MANIFEST_SOURCE_FILES = 'build/@(fonts|images|scripts|styles)/**/*.*';
 
 /**
  * Generate a JSON manifest mapping file paths to

diff --git a/h/assets.py b/h/assets.py
@@ -85,12 +85,16 @@ def urls(self, bundle):
         Returns the URLs at which all files in a bundle are served,
         read from the asset manifest.
         """
-        manifest = self.manifest.load()
         bundles = self.bundles.load()
 
-        def asset_url(path):
-            return '{}/{}'.format(self.assets_base_url, manifest[path])
-        return [asset_url(path) for path in bundles[bundle]]
+        return [self.asset_url(path) for path in bundles[bundle]]
+
+    def asset_url(self, path):
+        """
+        Return the cache-busted URL for an asset with a given path.
+        """
+        manifest = self.manifest.load()
+        return '{}/{}'.format(self.assets_base_url, manifest[path])
 
 
 def _add_cors_header(wrapped):
@@ -147,3 +151,5 @@ def includeme(config):
     # integration can be configured in app.py
     config.registry['assets_env'] = assets_env
     config.registry['assets_client_env'] = assets_client_env
+
+    config.add_request_method(assets_env.asset_url, 'asset_url')
diff --git a/h/templates/5xx.html.jinja2 b/h/templates/5xx.html.jinja2
@@ -10,7 +10,7 @@
           font-family: "Helvetica Neue",Helvetica,Arial,"Lucida Grande",sans-serif;
           font-weight: 300;
           color: #585858;
-          background: #fff url(/assets/images/noise_1.png);
+          background: #fff url({{ request.asset_url('images/noise_1.png') }});
         }
 
         h1 {
@@ -58,7 +58,7 @@
   </head>
   <body>
   <main class="content paper styled-text page">
-    <img id="graphic" src="/assets/images/sad-annotation.svg" />
+    <img id="graphic" src="{{ request.asset_url('images/sad-annotation.svg') }}" />
     <h1>Uh-oh, something went wrong!</h1>
     <p>We&rsquo;re very sorry, our application wasn&rsquo;t able to load this page. The
        team has been notified and we&rsquo;ll&nbsp;fix&nbsp;it&nbsp;shortly.</p>

diff --git a/h/templates/groups/join.html.jinja2 b/h/templates/groups/join.html.jinja2
@@ -41,7 +41,7 @@
   {% else %}
     <div class="content content--narrow">
       <div class="group-form">
-        <img class="group-form__invite-icon" src="/assets/images/icons/group-invite.svg">
+        <img class="group-form__invite-icon" src="{{ request.asset_url('images/icons/group-invite.svg') }}">
         <div class="group-form__name-label">You have been invited to annotate with the group</div>
         <div class="group-form__name-input">{{ group.name }}</div>
         {% if request.authenticated_userid %}

diff --git a/h/templates/includes/logo-header.html.jinja2 b/h/templates/includes/logo-header.html.jinja2
@@ -1,7 +1,7 @@
 <header class="masthead">
   {% if feature('activity_pages') %}
   <a href="/" title="Hypothesis homepage"><!--
-    !--><img alt="Hypothesis logo" class="masthead-logo" src="/assets/images/logo.svg"></a>
+    !--><img alt="Hypothesis logo" class="masthead-logo" src="{{ request.asset_url('images/logo.svg') }}"></a>
   {% else %}
   <hgroup>
 	<a href="https://hypothes.is" class="masthead-heading">Hypothes<span class="red">.</span>is</a>

diff --git a/h/templates/panels/navbar.html.jinja2 b/h/templates/panels/navbar.html.jinja2
@@ -30,7 +30,8 @@
   </template>
   <div class="nav-bar__content">
     <a href="/" title="Hypothesis homepage" class="nav-bar__logo-container"><!--
-      !--><img alt="Hypothesis logo" class="nav-bar__logo" src="/assets/images/logo.svg"></a>
+      !--><img alt="Hypothesis logo" class="nav-bar__logo"
+               src="{{ request.asset_url('images/logo.svg') }}"></a>
 
     <div class="nav-bar__search js-search-bar" data-ref="searchBar">
       <form class="search-bar"

diff --git a/tests/h/assets_test.py b/tests/h/assets_test.py
@@ -58,6 +58,14 @@ def test_environment_generates_bundle_urls(mtime):
     ]
 
 
+@patch(open_target, _fake_open)
+@patch('os.path.getmtime')
+def test_environment_asset_url_returns_url(mtime):
+    env = Environment('/assets', 'bundles.ini', 'manifest.json')
+
+    assert env.asset_url('app.bundle.js') == '/assets/app.bundle.js?abcdef'
+
+
 @patch(open_target)
 @patch('os.path.getmtime')
 def test_environment_reloads_manifest_on_change(mtime, open):