From 362f405492853f591d9060a144103f89c25ecebc Mon Sep 17 00:00:00 2001 From: bloodearnest Date: Mon, 18 Mar 2024 17:06:14 +0000 Subject: [PATCH] Add immutable cache ids to contents urls And add far future cache headers Seems to work a treat! --- airlock/business_logic.py | 15 ++++++++++++++- airlock/renderers.py | 6 ++++-- tests/integration/views/test_workspace.py | 4 ++-- tests/unit/test_business_logic.py | 10 ++++++---- tests/unit/test_file_browser_api.py | 8 +++----- tests/unit/test_renderers.py | 2 ++ 6 files changed, 31 insertions(+), 14 deletions(-) diff --git a/airlock/business_logic.py b/airlock/business_logic.py index 6b2f6a3d3..3664fc03d 100644 --- a/airlock/business_logic.py +++ b/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 @@ -96,11 +97,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 @@ -208,6 +215,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): diff --git a/airlock/renderers.py b/airlock/renderers.py index 5c26472fa..d4726ae5c 100644 --- a/airlock/renderers.py +++ b/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}", } diff --git a/tests/integration/views/test_workspace.py b/tests/integration/views/test_workspace.py index 09762d042..f3bbe2487 100644 --- a/tests/integration/views/test_workspace.py +++ b/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): diff --git a/tests/unit/test_business_logic.py b/tests/unit/test_business_logic.py index 54655ad52..feea03d05 100644 --- a/tests/unit/test_business_logic.py +++ b/tests/unit/test_business_logic.py @@ -15,6 +15,7 @@ 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" @@ -22,13 +23,14 @@ def test_workspace_container(): 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") ) 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" @@ -36,8 +38,8 @@ def test_request_container(): 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") ) diff --git a/tests/unit/test_file_browser_api.py b/tests/unit/test_file_browser_api.py index 131fd5882..90eb0137c 100644 --- a/tests/unit/test_file_browser_api.py +++ b/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() diff --git a/tests/unit/test_renderers.py b/tests/unit/test_renderers.py index 66d8b48e0..5d808801b 100644 --- a/tests/unit/test_renderers.py +++ b/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"