diff --git a/gramps_webapi/api/cache.py b/gramps_webapi/api/cache.py index e227569c..7c72d34a 100644 --- a/gramps_webapi/api/cache.py +++ b/gramps_webapi/api/cache.py @@ -6,13 +6,12 @@ import json import os -from flask import request +from flask import g, request from flask_caching import Cache from gramps.gen.errors import HandleError from gramps_webapi.api.auth import has_permissions from gramps_webapi.api.util import ( - abort_with_message, get_db_handle, get_db_manager, get_tree_from_jwt, @@ -60,14 +59,8 @@ def make_cache_key_thumbnails(*args, **kwargs): # Checksum comes from the DB, not the query parameter (which is only a # frontend service worker cache-busting hint and is excluded from arg_hash). - handle = kwargs["handle"] tree = get_tree_from_jwt() - db_handle = get_db_handle() - try: - obj = db_handle.get_media_from_handle(handle) - except HandleError: - abort_with_message(404, f"Handle {handle} not found") - checksum = obj.checksum + checksum = g.cached_media.checksum dbmgr = get_db_manager(tree) @@ -106,11 +99,24 @@ def skip_cache_condition_request(*args, **kwargs) -> bool: return should_skip +def skip_cache_missing_media(*args, **kwargs) -> bool: + """Look up the media object, and skip caching when it does not exist. + + Runs before the cache key is made, so the key functions can rely on + `g.cached_media` and the view is left to raise the 404. + """ + try: + g.cached_media = get_db_handle().get_media_from_handle(kwargs["handle"]) + except HandleError: + return True + return False + + request_cache_decorator = request_cache.cached( make_cache_key=make_cache_key_request, unless=skip_cache_condition_request ) thumbnail_cache_decorator = thumbnail_cache.cached( - make_cache_key=make_cache_key_thumbnails + make_cache_key=make_cache_key_thumbnails, unless=skip_cache_missing_media ) @@ -120,13 +126,8 @@ def make_cache_key_tiles(*args, **kwargs): # jwt and checksum are excluded by _hash_request_args as usual. arg_hash = _hash_request_args() - handle = kwargs["handle"] tree = get_tree_from_jwt() - db_handle = get_db_handle() - try: - obj = db_handle.get_media_from_handle(handle) - except HandleError: - abort_with_message(404, f"Handle {handle} not found") + obj = g.cached_media checksum = obj.checksum # Include a hash of map:bounds so that updating the attribute (without @@ -155,7 +156,7 @@ def make_cache_key_tiles(*args, **kwargs): tile_cache_decorator = thumbnail_cache.cached( - make_cache_key=make_cache_key_tiles + make_cache_key=make_cache_key_tiles, unless=skip_cache_missing_media ) diff --git a/tests/test_endpoints/test_file.py b/tests/test_endpoints/test_file.py index bcae1439..d16ae14e 100644 --- a/tests/test_endpoints/test_file.py +++ b/tests/test_endpoints/test_file.py @@ -109,6 +109,17 @@ def test_get_thumbnail_with_checksum(self): ) assert rv.data == rv_checksum.data + def test_get_thumbnail_unknown_handle(self): + """A missing media object is a plain 404, not a cache backend failure. + + Aborting while the cache key is computed would be swallowed and logged + by flask_caching instead of reaching the client as an error. + """ + header = fetch_header(self.client) + with self.assertNoLogs("flask_caching", level="ERROR"): + rv = self.client.get(f"{TEST_URL}does_not_exist/thumbnail/20", headers=header) + assert rv.status_code == 404 + def test_get_thumbnail_large_requires_token(self): """Test authorization required.""" check_requires_token(self, TEST_URL + "b39fe1cfc1305ac4a21/thumbnail/10000") @@ -312,6 +323,19 @@ def test_get_map_tile_requires_token(self): rv = self.client.get(TEST_URL + "b39fe1cfc1305ac4a21/tile/5/16/11", headers=header) self.assertNotEqual(rv.status_code, 500) + def test_get_map_tile_unknown_handle(self): + """A missing media object is a plain 404, not a cache backend failure. + + Aborting while the cache key is computed would be swallowed and logged + by flask_caching instead of reaching the client as an error. + """ + header = fetch_header(self.client) + with self.assertNoLogs("flask_caching", level="ERROR"): + rv = self.client.get( + f"{TEST_URL}does_not_exist/tile/5/16/11", headers=header + ) + assert rv.status_code == 404 + def test_get_map_tile_no_bounds_returns_404(self): """Media without map:bounds attribute returns 404.""" media_objects = check_success(self, TEST_URL)