Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 18 additions & 17 deletions gramps_webapi/api/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
)


Expand All @@ -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
Expand Down Expand Up @@ -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
)


Expand Down
11 changes: 11 additions & 0 deletions tests/test_endpoints/test_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
Comment thread
DavidMStraub marked this conversation as resolved.
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")
Expand Down