Add immutable cache ids to contents urls

And add far future cache headers

Seems to work a treat!

airlock/business_logic.py

@@ -16,6 +16,7 @@
 from django.utils.module_loading import import_string
 
 import old_api
+from airlock.renderers import get_renderer
 from airlock.users import User
 
 
@@ -104,11 +105,17 @@ def get_url(self, relpath=ROOT_PATH):
         )
 
     def get_contents_url(self, relpath, download=False):
-        return reverse(
+        url = reverse(
             "workspace_contents",
             kwargs={"workspace_name": self.name, "path": relpath},
         )
 
+        # what renderer would render this file?
+        renderer = get_renderer(self.abspath(relpath))
+        url += f"?cache_id={renderer.cache_id}"
+
+        return url
+
     def abspath(self, relpath):
         """Get absolute path for file
 
@@ -230,6 +237,12 @@ def get_contents_url(self, relpath, download=False):
         )
         if download:
             url += "?download"
+        else:
+            # what renderer would render this file?
+            request_file = self.get_request_file(relpath)
+            renderer = get_renderer(self.abspath(relpath), request_file)
+            url += f"?cache_id={renderer.cache_id}"
+
         return url
 
     def get_request_file(self, relpath: UrlPath | str):

airlock/renderers.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import csv
 from dataclasses import dataclass
 from email.utils import formatdate
@@ -8,8 +10,6 @@
 from django.template import Template, loader
 from django.template.response import SimpleTemplateResponse
 
-from airlock.business_logic import RequestFile
-
 
 @dataclass
 class RendererTemplate:
@@ -27,6 +27,7 @@ def cache_id(self):
 
 @dataclass
 class Renderer:
+    MAX_AGE = 365 * 24 * 60 * 60  # 1 year
     template = None
 
     abspath: Path
@@ -66,6 +67,7 @@ def headers(self):
         return {
             "ETag": self.etag,
             "Last-Modified": self.last_modified,
+            "Cache-Control": f"max-age={self.MAX_AGE}",
         }
 
 

tests/integration/views/test_workspace.py

@@ -84,7 +84,7 @@ def test_workspace_view_with_html_file(airlock_client):
         "workspace_contents",
         kwargs={"workspace_name": "workspace", "path": "file.html"},
     )
-    assert f'src="{url}"' in response.rendered_content
+    assert f"{url}?cache_id=" in response.rendered_content
 
 
 def test_workspace_view_with_svg_file(airlock_client):
@@ -124,7 +124,7 @@ def test_workspace_view_with_svg_file(airlock_client):
         "workspace_contents",
         kwargs={"workspace_name": "workspace", "path": "file.svg"},
     )
-    assert f'src="{url}"' in response.rendered_content
+    assert f"{url}?cache_id=" in response.rendered_content
 
 
 def test_workspace_view_with_csv_file(airlock_client):

tests/unit/test_business_logic.py

@@ -21,15 +21,16 @@
 
 def test_workspace_container():
     workspace = factories.create_workspace("workspace")
+    factories.write_workspace_file(workspace, "foo/bar.html")
 
     assert workspace.root() == settings.WORKSPACE_DIR / "workspace"
     assert workspace.get_id() == "workspace"
     assert (
         workspace.get_url("foo/bar.html") == "/workspaces/view/workspace/foo/bar.html"
     )
     assert (
-        workspace.get_contents_url("foo/bar.html")
-        == "/workspaces/content/workspace/foo/bar.html"
+        "/workspaces/content/workspace/foo/bar.html?cache_id="
+        in workspace.get_contents_url("foo/bar.html")
     )
 
 
@@ -41,15 +42,16 @@ def test_workspace_is_supporting_file(bll):
 
 def test_request_container():
     release_request = factories.create_release_request("workspace", id="id")
+    factories.write_request_file(release_request, "group", "bar.html")
 
     assert release_request.root() == settings.REQUEST_DIR / "workspace/id"
     assert release_request.get_id() == "id"
     assert (
         release_request.get_url("group/bar.html") == "/requests/view/id/group/bar.html"
     )
     assert (
-        release_request.get_contents_url("group/bar.html")
-        == "/requests/content/id/group/bar.html"
+        "/requests/content/id/group/bar.html?cache_id="
+        in release_request.get_contents_url("group/bar.html")
     )
 
 

tests/unit/test_file_browser_api.py

@@ -218,11 +218,9 @@ def test_workspace_tree_urls(workspace, path, url):
 
 def test_workspace_tree_content_urls(workspace):
     tree = get_workspace_tree(workspace)
-    assert (
-        tree.get_path("some_dir/file_a.txt")
-        .contents_url()
-        .endswith("some_dir/file_a.txt")
-    )
+    url = tree.get_path("some_dir/file_a.txt").contents_url()
+    assert "some_dir/file_a.txt" in url
+    assert "cache_id=" in url
 
     with pytest.raises(Exception):
         assert tree.get_path("some_dir").contents_url()

tests/unit/test_renderers.py

@@ -45,6 +45,7 @@ def test_renderers_get_renderer_workspace(
     assert response.headers["Content-Type"].split(";")[0] == mimetype
     assert response.headers["Last-Modified"] == renderer.last_modified
     assert response.headers["ETag"] == renderer.etag
+    assert response.headers["Cache-Control"] == "max-age=31536000"
 
 
 @pytest.mark.parametrize("suffix,mimetype,template_path", RENDERER_TESTS)
@@ -79,3 +80,4 @@ def test_renderers_get_renderer_request(tmp_path, rf, suffix, mimetype, template
     assert response.headers["Content-Type"].split(";")[0] == mimetype
     assert response.headers["Last-Modified"] == renderer.last_modified
     assert response.headers["ETag"] == renderer.etag
+    assert response.headers["Cache-Control"] == "max-age=31536000"