diff options
| -rw-r--r-- | Justfile | 2 | ||||
| -rwxr-xr-x | bin/shuriken | 240 | ||||
| -rw-r--r-- | src/lib/album-metadata.source.sh | 91 | ||||
| -rw-r--r-- | src/lib/album-render.source.sh | 27 | ||||
| -rw-r--r-- | src/lib/metadata-cache.source.sh | 104 | ||||
| -rw-r--r-- | src/lib/stats-aggregate.source.sh | 3 | ||||
| -rw-r--r-- | src/lib/stats-filter-album.source.sh | 13 | ||||
| -rwxr-xr-x | tests/cli.sh | 58 |
8 files changed, 343 insertions, 195 deletions
@@ -7,7 +7,7 @@ PREFIX := env_var_or_default("PREFIX", "/usr") BINDIR := env_var_or_default("BINDIR", PREFIX + "/bin") DATADIR := env_var_or_default("DATADIR", PREFIX + "/share") SYSCONFDIR := env_var_or_default("SYSCONFDIR", "/etc/default") -LIB_SOURCES := "src/lib/logging.source.sh src/lib/bootstrap.source.sh src/lib/paths.source.sh src/lib/imagemagick.source.sh src/lib/process.source.sh src/lib/archive.source.sh src/lib/template.source.sh src/lib/job-pool.source.sh src/lib/image.source.sh src/lib/random.source.sh src/lib/metadata-label.source.sh src/lib/image-pipeline.source.sh src/lib/album-metadata.source.sh src/lib/album-render.source.sh src/lib/album.source.sh src/lib/stats-aggregate.source.sh src/lib/stats-render.source.sh src/lib/stats-filter-album.source.sh src/lib/config.source.sh src/lib/config.print.source.sh src/lib/config.sync.source.sh src/lib/config.staging.source.sh src/lib/config.validate.source.sh src/lib/config.cli.source.sh src/lib/action.source.sh" +LIB_SOURCES := "src/lib/logging.source.sh src/lib/bootstrap.source.sh src/lib/paths.source.sh src/lib/imagemagick.source.sh src/lib/process.source.sh src/lib/archive.source.sh src/lib/template.source.sh src/lib/job-pool.source.sh src/lib/image.source.sh src/lib/random.source.sh src/lib/metadata-label.source.sh src/lib/metadata-cache.source.sh src/lib/image-pipeline.source.sh src/lib/album-metadata.source.sh src/lib/album-render.source.sh src/lib/album.source.sh src/lib/stats-aggregate.source.sh src/lib/stats-render.source.sh src/lib/stats-filter-album.source.sh src/lib/config.source.sh src/lib/config.print.source.sh src/lib/config.sync.source.sh src/lib/config.staging.source.sh src/lib/config.validate.source.sh src/lib/config.cli.source.sh src/lib/action.source.sh" default: build diff --git a/bin/shuriken b/bin/shuriken index f0ee7b8..c251336 100755 --- a/bin/shuriken +++ b/bin/shuriken @@ -1788,6 +1788,112 @@ camera_label_from_make_model() { esac } +# Inlined from src/lib/metadata-cache.source.sh +# Shared EXIF identify cache primitive. Promoted out of album-metadata.source.sh +# (task pn0) because the cached `identify -verbose` reader is a low-level +# metadata primitive consumed by BOTH the album (tooltips, details tables) and +# the stats aggregator (leaderboard tallies). Keeping it inside the album module +# forced stats to reach across a module boundary into album internals; moving it +# here gives both consumers a shared, stable dependency that is sourced before +# either of them (see LIB_SOURCES in the Justfile, ordered right after +# metadata-label.source.sh, the sibling shared metadata helper). All library +# modules are sourced before any code runs, so source order documents the +# dependency, it does not affect availability. Behaviour and signatures are +# unchanged by the move. + +# Build the cache signature line ("<photo>:<size>:<mtime>") used to decide +# whether a cache entry is still valid for the source file. Kept private to this +# module alongside its only consumers, plus the stats test that pre-seeds caches. +photo_cache_signature() { + local -r photo="$1"; shift + local -r photo_path="$1"; shift + local stat_output + + stat_output=$(stat -c '%s:%Y' "$photo_path") + printf '%s:%s\n' "$photo" "$stat_output" +} + +# Print a cache file's payload (everything after the leading signature line). +print_cached_photo_identify_output() { + local -r cache_file="$1"; shift + local line + local skipped_signature=no + + while IFS= read -r line || [ -n "$line" ]; do + if [ "$skipped_signature" = no ]; then + skipped_signature=yes + continue + fi + printf '%s\n' "$line" + done < "$cache_file" +} + +# Return cached ImageMagick `identify -verbose` output for a photo, rebuilding +# the cache when missing or stale. Public shared primitive: album-metadata and +# stats-aggregate both call this rather than running identify themselves. +cached_photo_identify_output() { + local -r photo="$1"; shift + local -r photo_path="$1"; shift + local cache_dir + local cache_file + local cached_signature='' + local current_signature + local identify_status + + # Persist the EXIF cache in a volatile ./cache directory parallel to ./dist + # (the staging dir is a sibling of the final dist, so dirname "$DIST_DIR" is + # the working dir in both staging and direct contexts). Keeping it outside + # dist means it survives a fresh/cleared dist and is never deployed, so an + # unchanged photo skips the slow `identify -verbose` on every regenerate. + cache_dir="$(dirname "$DIST_DIR")/cache/exif" + cache_file="$cache_dir/$photo.txt" + current_signature=$(photo_cache_signature "$photo" "$photo_path") + + # Reuse the cache when its signature still matches the source file. --force + # is handled once up front by clear_exif_cache (which empties this directory), + # so the first call per photo then rebuilds it and the rest of the run reuses + # it -- exactly one identify per photo even under force. + if [ -f "$cache_file" ]; then + IFS= read -r cached_signature < "$cache_file" || true + if [ "$cached_signature" = "$current_signature" ]; then + print_cached_photo_identify_output "$cache_file" + return + fi + fi + + mkdir -p "$cache_dir" + printf '%s\n' "$current_signature" > "$cache_file" + + # Capture the identify exit status instead of swallowing it with `|| true`. + # Errors are still hidden from stdout (so a corrupt photo does not pollute + # the EXIF output), but a non-zero status now drives a warning + no-cache + # rather than silently leaving a signature-only cache entry behind. + identify_status=0 + imagemagick_identify -verbose "$photo_path" >> "$cache_file" 2>/dev/null \ + || identify_status=$? + + if [ "$identify_status" -ne 0 ]; then + # Failed identify (corrupt photo, timeout, missing binary, ...): warn + # naming the photo and remove the cache file. Removing it is essential: + # a file holding only the signature line is a valid-looking cache hit, + # so the next run would silently reuse the empty result forever -- never + # retrying identify and never warning again (the original data-loss bug). + # Deleting it makes the next run retry and warn. + # + # We deliberately do NOT abort: this runs inside backgrounded render jobs + # under `set -euo pipefail`, and one unreadable photo must not kill the + # whole generation. The photo still renders, just with empty tooltip and + # stats, now accompanied by a warning. + rm -f "$cache_file" + log_warning \ + "could not read EXIF for $photo (ImageMagick identify failed);" \ + "tooltip/stats will be missing" + return 0 + fi + + print_cached_photo_identify_output "$cache_file" +} + # Inlined from src/lib/image-pipeline.source.sh create_photo_derivatives() { local -r photos_dir="$1"; shift @@ -1875,91 +1981,12 @@ prepare_generation_photo_assets() { } # Inlined from src/lib/album-metadata.source.sh -photo_cache_signature() { - local -r photo="$1"; shift - local -r photo_path="$1"; shift - local stat_output - - stat_output=$(stat -c '%s:%Y' "$photo_path") - printf '%s:%s\n' "$photo" "$stat_output" -} - -print_cached_photo_identify_output() { - local -r cache_file="$1"; shift - local line - local skipped_signature=no - - while IFS= read -r line || [ -n "$line" ]; do - if [ "$skipped_signature" = no ]; then - skipped_signature=yes - continue - fi - printf '%s\n' "$line" - done < "$cache_file" -} - -cached_photo_identify_output() { - local -r photo="$1"; shift - local -r photo_path="$1"; shift - local cache_dir - local cache_file - local cached_signature='' - local current_signature - local identify_status - - # Persist the EXIF cache in a volatile ./cache directory parallel to ./dist - # (the staging dir is a sibling of the final dist, so dirname "$DIST_DIR" is - # the working dir in both staging and direct contexts). Keeping it outside - # dist means it survives a fresh/cleared dist and is never deployed, so an - # unchanged photo skips the slow `identify -verbose` on every regenerate. - cache_dir="$(dirname "$DIST_DIR")/cache/exif" - cache_file="$cache_dir/$photo.txt" - current_signature=$(photo_cache_signature "$photo" "$photo_path") - - # Reuse the cache when its signature still matches the source file. --force - # is handled once up front by clear_exif_cache (which empties this directory), - # so the first call per photo then rebuilds it and the rest of the run reuses - # it -- exactly one identify per photo even under force. - if [ -f "$cache_file" ]; then - IFS= read -r cached_signature < "$cache_file" || true - if [ "$cached_signature" = "$current_signature" ]; then - print_cached_photo_identify_output "$cache_file" - return - fi - fi - - mkdir -p "$cache_dir" - printf '%s\n' "$current_signature" > "$cache_file" - - # Capture the identify exit status instead of swallowing it with `|| true`. - # Errors are still hidden from stdout (so a corrupt photo does not pollute - # the EXIF output), but a non-zero status now drives a warning + no-cache - # rather than silently leaving a signature-only cache entry behind. - identify_status=0 - imagemagick_identify -verbose "$photo_path" >> "$cache_file" 2>/dev/null \ - || identify_status=$? - - if [ "$identify_status" -ne 0 ]; then - # Failed identify (corrupt photo, timeout, missing binary, ...): warn - # naming the photo and remove the cache file. Removing it is essential: - # a file holding only the signature line is a valid-looking cache hit, - # so the next run would silently reuse the empty result forever -- never - # retrying identify and never warning again (the original data-loss bug). - # Deleting it makes the next run retry and warn. - # - # We deliberately do NOT abort: this runs inside backgrounded render jobs - # under `set -euo pipefail`, and one unreadable photo must not kill the - # whole generation. The photo still renders, just with empty tooltip and - # stats, now accompanied by a warning. - rm -f "$cache_file" - log_warning \ - "could not read EXIF for $photo (ImageMagick identify failed);" \ - "tooltip/stats will be missing" - return 0 - fi - - print_cached_photo_identify_output "$cache_file" -} +# The EXIF identify cache primitive (cached_photo_identify_output and its +# private helpers photo_cache_signature / print_cached_photo_identify_output) +# was promoted to the shared metadata-cache.source.sh module (task pn0): it is a +# low-level metadata primitive consumed by both this album module and the stats +# aggregator, so it no longer belongs to album internals. The helpers below call +# cached_photo_identify_output through that shared module. photo_exif_details_html() { local -r photo="$1"; shift @@ -2386,12 +2413,26 @@ print_dry_run_plan() { # Inlined from src/lib/album-render.source.sh # Maps each album photo filename to its view-page basename ("<page>-<preview>") -# as assigned during render_album_pages. The stats filter mini-albums read this -# so each view page can link "Details" to the album's own details page for the -# photo. Declared globally so it always exists for callers even when no album -# was rendered. +# as assigned during render_album_pages. Declared globally so it always exists +# for the accessor even when no album was rendered. +# +# This is the album module's PRIVATE backing store (task pn0). Outside callers +# must NOT index it directly: use the album_view_page_for_photo accessor below. +# Keeping the map private behind a documented function decouples consumers (the +# stats filter mini-albums) from how the album internally names or caches view +# pages, so a change to the page-naming scheme stays contained in this module. declare -gA ALBUM_VIEW_PAGE_BY_PHOTO=() +# Public album API (task pn0): return the view-page basename for a photo, or the +# empty string when the photo was not rendered into the album. The stats filter +# mini-albums call this to link each photo's "Details" to the album's own +# details page, instead of reaching into ALBUM_VIEW_PAGE_BY_PHOTO directly. +album_view_page_for_photo() { + local -r photo="$1"; shift + + printf '%s' "${ALBUM_VIEW_PAGE_BY_PHOTO[$photo]:-}" +} + album_photo_files() { local -r photos_dir="$1"; shift @@ -3003,8 +3044,9 @@ _album_record_view_photo() { "$pids_name" "$statuses_name" "$labels_name" "$failed_name" record_rendered_view_page "$view_pages_name" "$last_views_name" \ "$page_num" "$preview_num" - # Read later by the stats filter mini-albums (render_filter_pages) for - # their Details links; shellcheck cannot see that cross-function use. + # Read later through the album_view_page_for_photo accessor (e.g. by the + # stats filter mini-albums for their Details links); shellcheck cannot see + # that cross-function use. # shellcheck disable=SC2034 ALBUM_VIEW_PAGE_BY_PHOTO["$photo"]="$page_num-$preview_num" } @@ -3960,7 +4002,8 @@ accumulate_photo_stats() { } # Iterate the album's incoming photos, read each one's cached identify output via -# album.source.sh's cache helper, and aggregate it into the STATS_* globals. +# the shared metadata-cache.source.sh primitive (task pn0), and aggregate it into +# the STATS_* globals. # This is the entry point the render tasks call before reading the counters. collect_photo_exif_stats() { local photo @@ -4352,9 +4395,11 @@ render_stats_page() { # so this concern is separate from the EXIF aggregation (stats-aggregate.source.sh) # and the stats overview page (stats-render.source.sh). Every tallied bucket # becomes a clickable mini album under dist/stats/<pagebase>/. This module reads -# the STATS_FILTER_* globals filled by collect_photo_exif_stats plus the -# album-side ALBUM_VIEW_PAGE_BY_PHOTO map at runtime; all libs are sourced before -# run so cross-module references resolve. +# the STATS_FILTER_* globals filled by collect_photo_exif_stats and resolves each +# photo's album view page through the album_view_page_for_photo accessor (task +# pn0) instead of indexing the album's private ALBUM_VIEW_PAGE_BY_PHOTO global, +# so stats stays decoupled from album-internal page naming/caching. All libs are +# sourced before run so cross-module references resolve. # ---------------------------------------------------------------------------- # Filter mini-album pages @@ -4366,7 +4411,8 @@ render_stats_page() { # within that filter. The "--<index>" suffix cannot collide with another gallery # name because a pagebase never contains "--". All pages reuse the album's shared # photos/thumbs/blurs assets (only the HTML differs); view pages link "Details" -# to the album's own details page via ALBUM_VIEW_PAGE_BY_PHOTO. Pages render in +# to the album's own details page via the album_view_page_for_photo accessor. +# Pages render in # parallel through the shared job pool, throttled to IMAGE_JOBS. The galleries # reuse camera.tmpl and the view pages reuse cameraview.tmpl. @@ -4504,7 +4550,7 @@ _stats_build_filterview_body() { if [ -n "$tooltip" ]; then tooltip_attr=" title=\"$(_html_escape "$tooltip")\"" fi - view_page="${ALBUM_VIEW_PAGE_BY_PHOTO[$photo]:-}" + view_page=$(album_view_page_for_photo "$photo") if [ -n "$view_page" ]; then details_link=$(printf ' <a href="%s/%s-details.html">Details</a> |' \ "$backhref_html" "$view_page") diff --git a/src/lib/album-metadata.source.sh b/src/lib/album-metadata.source.sh index cc500ae..01bf826 100644 --- a/src/lib/album-metadata.source.sh +++ b/src/lib/album-metadata.source.sh @@ -1,88 +1,9 @@ -photo_cache_signature() { - local -r photo="$1"; shift - local -r photo_path="$1"; shift - local stat_output - - stat_output=$(stat -c '%s:%Y' "$photo_path") - printf '%s:%s\n' "$photo" "$stat_output" -} - -print_cached_photo_identify_output() { - local -r cache_file="$1"; shift - local line - local skipped_signature=no - - while IFS= read -r line || [ -n "$line" ]; do - if [ "$skipped_signature" = no ]; then - skipped_signature=yes - continue - fi - printf '%s\n' "$line" - done < "$cache_file" -} - -cached_photo_identify_output() { - local -r photo="$1"; shift - local -r photo_path="$1"; shift - local cache_dir - local cache_file - local cached_signature='' - local current_signature - local identify_status - - # Persist the EXIF cache in a volatile ./cache directory parallel to ./dist - # (the staging dir is a sibling of the final dist, so dirname "$DIST_DIR" is - # the working dir in both staging and direct contexts). Keeping it outside - # dist means it survives a fresh/cleared dist and is never deployed, so an - # unchanged photo skips the slow `identify -verbose` on every regenerate. - cache_dir="$(dirname "$DIST_DIR")/cache/exif" - cache_file="$cache_dir/$photo.txt" - current_signature=$(photo_cache_signature "$photo" "$photo_path") - - # Reuse the cache when its signature still matches the source file. --force - # is handled once up front by clear_exif_cache (which empties this directory), - # so the first call per photo then rebuilds it and the rest of the run reuses - # it -- exactly one identify per photo even under force. - if [ -f "$cache_file" ]; then - IFS= read -r cached_signature < "$cache_file" || true - if [ "$cached_signature" = "$current_signature" ]; then - print_cached_photo_identify_output "$cache_file" - return - fi - fi - - mkdir -p "$cache_dir" - printf '%s\n' "$current_signature" > "$cache_file" - - # Capture the identify exit status instead of swallowing it with `|| true`. - # Errors are still hidden from stdout (so a corrupt photo does not pollute - # the EXIF output), but a non-zero status now drives a warning + no-cache - # rather than silently leaving a signature-only cache entry behind. - identify_status=0 - imagemagick_identify -verbose "$photo_path" >> "$cache_file" 2>/dev/null \ - || identify_status=$? - - if [ "$identify_status" -ne 0 ]; then - # Failed identify (corrupt photo, timeout, missing binary, ...): warn - # naming the photo and remove the cache file. Removing it is essential: - # a file holding only the signature line is a valid-looking cache hit, - # so the next run would silently reuse the empty result forever -- never - # retrying identify and never warning again (the original data-loss bug). - # Deleting it makes the next run retry and warn. - # - # We deliberately do NOT abort: this runs inside backgrounded render jobs - # under `set -euo pipefail`, and one unreadable photo must not kill the - # whole generation. The photo still renders, just with empty tooltip and - # stats, now accompanied by a warning. - rm -f "$cache_file" - log_warning \ - "could not read EXIF for $photo (ImageMagick identify failed);" \ - "tooltip/stats will be missing" - return 0 - fi - - print_cached_photo_identify_output "$cache_file" -} +# The EXIF identify cache primitive (cached_photo_identify_output and its +# private helpers photo_cache_signature / print_cached_photo_identify_output) +# was promoted to the shared metadata-cache.source.sh module (task pn0): it is a +# low-level metadata primitive consumed by both this album module and the stats +# aggregator, so it no longer belongs to album internals. The helpers below call +# cached_photo_identify_output through that shared module. photo_exif_details_html() { local -r photo="$1"; shift diff --git a/src/lib/album-render.source.sh b/src/lib/album-render.source.sh index 9bae930..e5817f3 100644 --- a/src/lib/album-render.source.sh +++ b/src/lib/album-render.source.sh @@ -1,10 +1,24 @@ # Maps each album photo filename to its view-page basename ("<page>-<preview>") -# as assigned during render_album_pages. The stats filter mini-albums read this -# so each view page can link "Details" to the album's own details page for the -# photo. Declared globally so it always exists for callers even when no album -# was rendered. +# as assigned during render_album_pages. Declared globally so it always exists +# for the accessor even when no album was rendered. +# +# This is the album module's PRIVATE backing store (task pn0). Outside callers +# must NOT index it directly: use the album_view_page_for_photo accessor below. +# Keeping the map private behind a documented function decouples consumers (the +# stats filter mini-albums) from how the album internally names or caches view +# pages, so a change to the page-naming scheme stays contained in this module. declare -gA ALBUM_VIEW_PAGE_BY_PHOTO=() +# Public album API (task pn0): return the view-page basename for a photo, or the +# empty string when the photo was not rendered into the album. The stats filter +# mini-albums call this to link each photo's "Details" to the album's own +# details page, instead of reaching into ALBUM_VIEW_PAGE_BY_PHOTO directly. +album_view_page_for_photo() { + local -r photo="$1"; shift + + printf '%s' "${ALBUM_VIEW_PAGE_BY_PHOTO[$photo]:-}" +} + album_photo_files() { local -r photos_dir="$1"; shift @@ -616,8 +630,9 @@ _album_record_view_photo() { "$pids_name" "$statuses_name" "$labels_name" "$failed_name" record_rendered_view_page "$view_pages_name" "$last_views_name" \ "$page_num" "$preview_num" - # Read later by the stats filter mini-albums (render_filter_pages) for - # their Details links; shellcheck cannot see that cross-function use. + # Read later through the album_view_page_for_photo accessor (e.g. by the + # stats filter mini-albums for their Details links); shellcheck cannot see + # that cross-function use. # shellcheck disable=SC2034 ALBUM_VIEW_PAGE_BY_PHOTO["$photo"]="$page_num-$preview_num" } diff --git a/src/lib/metadata-cache.source.sh b/src/lib/metadata-cache.source.sh new file mode 100644 index 0000000..45edd3c --- /dev/null +++ b/src/lib/metadata-cache.source.sh @@ -0,0 +1,104 @@ +# Shared EXIF identify cache primitive. Promoted out of album-metadata.source.sh +# (task pn0) because the cached `identify -verbose` reader is a low-level +# metadata primitive consumed by BOTH the album (tooltips, details tables) and +# the stats aggregator (leaderboard tallies). Keeping it inside the album module +# forced stats to reach across a module boundary into album internals; moving it +# here gives both consumers a shared, stable dependency that is sourced before +# either of them (see LIB_SOURCES in the Justfile, ordered right after +# metadata-label.source.sh, the sibling shared metadata helper). All library +# modules are sourced before any code runs, so source order documents the +# dependency, it does not affect availability. Behaviour and signatures are +# unchanged by the move. + +# Build the cache signature line ("<photo>:<size>:<mtime>") used to decide +# whether a cache entry is still valid for the source file. Kept private to this +# module alongside its only consumers, plus the stats test that pre-seeds caches. +photo_cache_signature() { + local -r photo="$1"; shift + local -r photo_path="$1"; shift + local stat_output + + stat_output=$(stat -c '%s:%Y' "$photo_path") + printf '%s:%s\n' "$photo" "$stat_output" +} + +# Print a cache file's payload (everything after the leading signature line). +print_cached_photo_identify_output() { + local -r cache_file="$1"; shift + local line + local skipped_signature=no + + while IFS= read -r line || [ -n "$line" ]; do + if [ "$skipped_signature" = no ]; then + skipped_signature=yes + continue + fi + printf '%s\n' "$line" + done < "$cache_file" +} + +# Return cached ImageMagick `identify -verbose` output for a photo, rebuilding +# the cache when missing or stale. Public shared primitive: album-metadata and +# stats-aggregate both call this rather than running identify themselves. +cached_photo_identify_output() { + local -r photo="$1"; shift + local -r photo_path="$1"; shift + local cache_dir + local cache_file + local cached_signature='' + local current_signature + local identify_status + + # Persist the EXIF cache in a volatile ./cache directory parallel to ./dist + # (the staging dir is a sibling of the final dist, so dirname "$DIST_DIR" is + # the working dir in both staging and direct contexts). Keeping it outside + # dist means it survives a fresh/cleared dist and is never deployed, so an + # unchanged photo skips the slow `identify -verbose` on every regenerate. + cache_dir="$(dirname "$DIST_DIR")/cache/exif" + cache_file="$cache_dir/$photo.txt" + current_signature=$(photo_cache_signature "$photo" "$photo_path") + + # Reuse the cache when its signature still matches the source file. --force + # is handled once up front by clear_exif_cache (which empties this directory), + # so the first call per photo then rebuilds it and the rest of the run reuses + # it -- exactly one identify per photo even under force. + if [ -f "$cache_file" ]; then + IFS= read -r cached_signature < "$cache_file" || true + if [ "$cached_signature" = "$current_signature" ]; then + print_cached_photo_identify_output "$cache_file" + return + fi + fi + + mkdir -p "$cache_dir" + printf '%s\n' "$current_signature" > "$cache_file" + + # Capture the identify exit status instead of swallowing it with `|| true`. + # Errors are still hidden from stdout (so a corrupt photo does not pollute + # the EXIF output), but a non-zero status now drives a warning + no-cache + # rather than silently leaving a signature-only cache entry behind. + identify_status=0 + imagemagick_identify -verbose "$photo_path" >> "$cache_file" 2>/dev/null \ + || identify_status=$? + + if [ "$identify_status" -ne 0 ]; then + # Failed identify (corrupt photo, timeout, missing binary, ...): warn + # naming the photo and remove the cache file. Removing it is essential: + # a file holding only the signature line is a valid-looking cache hit, + # so the next run would silently reuse the empty result forever -- never + # retrying identify and never warning again (the original data-loss bug). + # Deleting it makes the next run retry and warn. + # + # We deliberately do NOT abort: this runs inside backgrounded render jobs + # under `set -euo pipefail`, and one unreadable photo must not kill the + # whole generation. The photo still renders, just with empty tooltip and + # stats, now accompanied by a warning. + rm -f "$cache_file" + log_warning \ + "could not read EXIF for $photo (ImageMagick identify failed);" \ + "tooltip/stats will be missing" + return 0 + fi + + print_cached_photo_identify_output "$cache_file" +} diff --git a/src/lib/stats-aggregate.source.sh b/src/lib/stats-aggregate.source.sh index af047c2..6b30f62 100644 --- a/src/lib/stats-aggregate.source.sh +++ b/src/lib/stats-aggregate.source.sh @@ -660,7 +660,8 @@ accumulate_photo_stats() { } # Iterate the album's incoming photos, read each one's cached identify output via -# album.source.sh's cache helper, and aggregate it into the STATS_* globals. +# the shared metadata-cache.source.sh primitive (task pn0), and aggregate it into +# the STATS_* globals. # This is the entry point the render tasks call before reading the counters. collect_photo_exif_stats() { local photo diff --git a/src/lib/stats-filter-album.source.sh b/src/lib/stats-filter-album.source.sh index 2bd6c1b..3f55342 100644 --- a/src/lib/stats-filter-album.source.sh +++ b/src/lib/stats-filter-album.source.sh @@ -2,9 +2,11 @@ # so this concern is separate from the EXIF aggregation (stats-aggregate.source.sh) # and the stats overview page (stats-render.source.sh). Every tallied bucket # becomes a clickable mini album under dist/stats/<pagebase>/. This module reads -# the STATS_FILTER_* globals filled by collect_photo_exif_stats plus the -# album-side ALBUM_VIEW_PAGE_BY_PHOTO map at runtime; all libs are sourced before -# run so cross-module references resolve. +# the STATS_FILTER_* globals filled by collect_photo_exif_stats and resolves each +# photo's album view page through the album_view_page_for_photo accessor (task +# pn0) instead of indexing the album's private ALBUM_VIEW_PAGE_BY_PHOTO global, +# so stats stays decoupled from album-internal page naming/caching. All libs are +# sourced before run so cross-module references resolve. # ---------------------------------------------------------------------------- # Filter mini-album pages @@ -16,7 +18,8 @@ # within that filter. The "--<index>" suffix cannot collide with another gallery # name because a pagebase never contains "--". All pages reuse the album's shared # photos/thumbs/blurs assets (only the HTML differs); view pages link "Details" -# to the album's own details page via ALBUM_VIEW_PAGE_BY_PHOTO. Pages render in +# to the album's own details page via the album_view_page_for_photo accessor. +# Pages render in # parallel through the shared job pool, throttled to IMAGE_JOBS. The galleries # reuse camera.tmpl and the view pages reuse cameraview.tmpl. @@ -154,7 +157,7 @@ _stats_build_filterview_body() { if [ -n "$tooltip" ]; then tooltip_attr=" title=\"$(_html_escape "$tooltip")\"" fi - view_page="${ALBUM_VIEW_PAGE_BY_PHOTO[$photo]:-}" + view_page=$(album_view_page_for_photo "$photo") if [ -n "$view_page" ]; then details_link=$(printf ' <a href="%s/%s-details.html">Details</a> |' \ "$backhref_html" "$view_page") diff --git a/tests/cli.sh b/tests/cli.sh index 10e9b19..5ac6561 100755 --- a/tests/cli.sh +++ b/tests/cli.sh @@ -6104,6 +6104,61 @@ test_stats_collect_reads_cached_identify_output() { test::teardown } +# Boundary test for the album/stats decoupling (task pn0). Proves two things: +# 1) the album_view_page_for_photo accessor returns exactly what the private +# ALBUM_VIEW_PAGE_BY_PHOTO backing store holds (and "" for unknown photos), so +# stats can rely on it instead of indexing the global; and +# 2) the assembled bin/shuriken keeps the cache primitive in the shared +# metadata-cache module and the stats filter mini-album code no longer indexes +# ALBUM_VIEW_PAGE_BY_PHOTO directly. A regression that re-coupled the modules +# (moving the cache helper back into album, or re-indexing the global from +# stats) would fail this. +test_album_stats_decoupling_boundary() { + local generated + local cache_section + local stats_filter_section + + test::setup + test::source_shuriken_lib + + # 1) The accessor reflects the private backing store and is the public API. + # Seed the album's global directly here (shellcheck cannot see that the + # accessor reads it back), then assert the accessor returns it. + ALBUM_VIEW_PAGE_BY_PHOTO=() + # shellcheck disable=SC2034 + ALBUM_VIEW_PAGE_BY_PHOTO['shot.jpg']='2-3' + test "$(album_view_page_for_photo 'shot.jpg')" = '2-3' + test "$(album_view_page_for_photo 'missing.jpg')" = '' + + # 2) Structural assertions on the assembled script. + generated=$(<"$TEST_SHURIKEN") + + # The cache primitive must live in the shared metadata-cache module. + cache_section=$(awk ' + /^# Inlined from src\/lib\/metadata-cache.source.sh/ { keep=1; next } + /^# Inlined from / { keep=0 } + keep { print } + ' <<< "$generated") + test::assert_contains 'cached_photo_identify_output()' "$cache_section" + + # The stats filter mini-album code must reach the album only through the + # accessor, never by indexing the album's private global directly. + stats_filter_section=$(awk ' + /^# Inlined from src\/lib\/stats-filter-album.source.sh/ { keep=1; next } + /^# Inlined from / { keep=0 } + keep { print } + ' <<< "$generated") + # Literal needle: we look for the accessor call verbatim in the assembled + # script, so the "$photo" must stay unexpanded. + # shellcheck disable=SC2016 + test::assert_contains 'album_view_page_for_photo "$photo"' \ + "$stats_filter_section" + test::assert_not_contains 'ALBUM_VIEW_PAGE_BY_PHOTO[' \ + "$stats_filter_section" + + test::teardown +} + # STATS_CATEGORIES is the single source of truth (task en0): proves the reset, # the overview body builder, and the bucket ladders all derive from the registry, # so a category can no longer be defined in only one place. A regression that @@ -6568,6 +6623,9 @@ main() { 'stats collect reads cached identify output' \ test_stats_collect_reads_cached_identify_output test::run_case \ + 'album/stats decoupling boundary (pn0)' \ + test_album_stats_decoupling_boundary + test::run_case \ 'stats categories registry is single source of truth' \ test_stats_categories_registry_is_single_source_of_truth test::run_case \ |