fix: compute config_hash for thumbnail artifacts

This alleviates some possible dependency tracking race conditions when
the source image is updated between the time when the thumbnail
parameters are computed and when the thumbnail is actually
created (and build database updated.)

lektor/imagetools/thumbnail.py

@@ -2,7 +2,9 @@
 from __future__ import annotations
 
 import dataclasses
+import hashlib
 import io
+import json
 import math
 import posixpath
 import sys
@@ -185,6 +187,18 @@ def get_tag(self) -> str:
         """
         return self.format_info.get_thumbnail_tag(self)
 
+    def config_hash(self, **kwargs: str) -> str:
+        """Get a hash that capstures the state of the the thumbnail_params.
+
+        This is used to detect with the thumbnail artifact needs to be rebuilt.
+
+        Any kwargs will also contribute to the hash value.
+        """
+        config = dataclasses.asdict(self)
+        config["_extra"] = kwargs
+        serialized = json.dumps(config, ensure_ascii=True, sort_keys=True)
+        return hashlib.sha1(serialized.encode("ascii")).hexdigest()
+
 
 def _scale(x: int, num: float, denom: float) -> int:
     """Compute x * num / denom, rounded to integer.
@@ -384,6 +398,28 @@ def _create_thumbnail(
     return thumbnail
 
 
+# FIXME: move this?
+def _get_file_hash(ctx: Context, file_path: str | Path) -> str:
+    """Get a "hash" of a source file.
+
+    We want a fast-to-compute string that will likely change if the source image
+    changes. Here we construct a "hash" from the file's mtime and size.
+
+    We use the builder's PathCache if we can find one via the build context, since it
+    may already have cached the values we need.
+
+    """
+    # pylint: disable-next=import-outside-toplevel
+    from lektor.builder import FileInfo  # circ dep
+
+    if ctx.build_state is not None:
+        path_cache = ctx.build_state.path_cache
+        file_info: FileInfo = path_cache.get_file_info(file_path)
+    else:
+        file_info = FileInfo(ctx.pad.env, file_path)
+    return f"{file_info.size:d}:{file_info.mtime:d}"
+
+
 def _create_artifact(
     source_image: str | Path | SupportsRead[bytes],
     thumbnail_params: ThumbnailParams,
@@ -422,6 +458,10 @@ def make_image_thumbnail(
     """Helper method that can create thumbnails from within the build process
     of an artifact.
     """
+    # Get a hash for the source image early. This is used in computing a config_hash
+    # for the thumbnail.  We want to ensure that the thumbnail will be rebuilt if the
+    # source image changes any time after we start looking at it.
+    source_image_hash = _get_file_hash(ctx, source_image)
     image_info = get_image_info(source_image)
     if isinstance(image_info, UnknownImageInfo):
         raise RuntimeError("Cannot process unknown images")
@@ -467,6 +507,7 @@ def make_image_thumbnail(
     ctx.add_sub_artifact(
         artifact_name=dst_url_path,
         sources=[source_image],
+        config_hash=thumbnail_params.config_hash(sourch_image_hash=source_image_hash),
         build_func=partial(_create_artifact, source_image, thumbnail_params),
     )
 

tests/imagetools/test_thumbnail.py

@@ -3,14 +3,17 @@
 import dataclasses
 import html.parser
 import io
+import os
+import shutil
 from contextlib import contextmanager
 from pathlib import Path
-from unittest import mock
 
 import PIL
 import pytest
 from pytest import approx
 
+from lektor.context import Context
+from lektor.db import Image
 from lektor.imagetools.thumbnail import _compute_cropbox
 from lektor.imagetools.thumbnail import _convert_icc_profile_to_srgb
 from lektor.imagetools.thumbnail import _create_artifact
@@ -270,6 +273,13 @@ def dummy_jpg_path(tmp_path_factory):
     return dummy_jpg
 
 
+@pytest.fixture
+def ctx(builder):
+    build_state = builder.new_build_state()
+    with Context(build_state.new_artifact("dummy-artifact")) as ctx:
+        yield ctx
+
+
 @pytest.mark.parametrize(
     "source_url_path, kwargs, expected_size, thumbnail_url_path",
     [
@@ -318,16 +328,15 @@ def dummy_jpg_path(tmp_path_factory):
     ],
 )
 def test_make_image_thumbnail(
-    source_url_path, kwargs, expected_size, thumbnail_url_path, dummy_jpg_path
+    ctx, source_url_path, kwargs, expected_size, thumbnail_url_path, dummy_jpg_path
 ):
-    ctx = mock.Mock(name="ctx")
     thumbnail = make_image_thumbnail(ctx, dummy_jpg_path, source_url_path, **kwargs)
     assert (thumbnail.width, thumbnail.height) == expected_size
     assert thumbnail.url_path == thumbnail_url_path
     if "@" in thumbnail_url_path:
-        assert ctx.add_sub_artifact.call_count == 1
+        assert len(ctx.sub_artifacts) == 1
     else:
-        assert len(ctx.mock_calls) == 0  # no implicit upscale
+        assert len(ctx.sub_artifacts) == 0  # no implicit upscale
 
 
 @pytest.mark.parametrize(
@@ -337,18 +346,16 @@ def test_make_image_thumbnail(
         ({"width": 8, "mode": ThumbnailMode.CROP}, "requires both"),
     ],
 )
-def test_make_image_thumbnail_invalid_params(params, match, dummy_jpg_path):
-    ctx = mock.Mock(name="ctx")
+def test_make_image_thumbnail_invalid_params(ctx, params, match, dummy_jpg_path):
     with pytest.raises(ValueError, match=match):
         make_image_thumbnail(ctx, dummy_jpg_path, "/test.jpg", **params)
-    assert len(ctx.mock_calls) == 0
+    assert len(ctx.sub_artifacts) == 0
 
 
-def test_make_image_thumbnail_unknown_image_format(tmp_path):
-    ctx = mock.Mock(name="ctx")
+def test_make_image_thumbnail_unknown_image_format(ctx, tmp_path):
     with pytest.raises(RuntimeError, match="unknown"):
         make_image_thumbnail(ctx, NONIMAGE_FILE_PATH, "/test.jpg", width=80)
-    assert len(ctx.mock_calls) == 0
+    assert len(ctx.sub_artifacts) == 0
 
 
 @pytest.fixture
@@ -363,19 +370,18 @@ def svg_file(**kwargs):
     return svg_file
 
 
-def test_make_image_thumbnail_svg(dummy_svg_file):
+def test_make_image_thumbnail_svg(ctx, dummy_svg_file):
     svg_file = dummy_svg_file(width="400px", height="300px")
-    ctx = mock.Mock(name="ctx")
     thumbnail = make_image_thumbnail(
         ctx, svg_file, "/urlpath/dummy.svg", width=80, height=60
     )
     assert (thumbnail.width, thumbnail.height) == (80, 60)
     assert thumbnail.url_path == "/urlpath/dummy.svg"
+    assert len(ctx.sub_artifacts) == 0
 
 
-def test_make_image_thumbnail_svg_fit_mode_fails_if_missing_dim(dummy_svg_file):
+def test_make_image_thumbnail_svg_fit_mode_fails_if_missing_dim(ctx, dummy_svg_file):
     svg_file = dummy_svg_file(width="400px")  # missing height
-    ctx = mock.Mock(name="ctx")
     with pytest.raises(ValueError, match="Cannot determine aspect ratio"):
         make_image_thumbnail(ctx, svg_file, "/urlpath/dummy.svg", width=80, height=60)
 
@@ -385,6 +391,67 @@ def test_Thumbnail_str():
     assert str(thumbnail) == "image.jpg"
 
 
+class TestArtifactDependencyTracking:
+    @staticmethod
+    @pytest.fixture
+    def scratch_project_data(scratch_project_data):
+        # add two identical thumbnails to the page template
+        page_html = scratch_project_data / "templates/page.html"
+        with page_html.open("a") as fp:
+            fp.write(
+                """
+                {% set im = this.attachments.get('test.jpg') %}
+                <img src="{{ im.thumbnail(20) }}">
+                <img src="{{ im.thumbnail(20) }}">
+                """
+            )
+        shutil.copy(ICC_PROFILE_TEST_JPG, scratch_project_data / "content/test.jpg")
+        return scratch_project_data
+
+    @staticmethod
+    def build_page(builder) -> list[str]:
+        _, build_state = builder.build(builder.pad.root)
+        assert len(build_state.failed_artifacts) == 0
+        return [
+            os.path.basename(artifact.dst_filename)
+            for artifact in build_state.updated_artifacts
+        ]
+
+    def test(self, scratch_builder):
+        built = self.build_page(scratch_builder)
+        assert built == ["index.html", "test@20x20.jpg"]
+
+        # rebuild doesn't rebuild any more artifacts
+        Path(scratch_builder.destination_path, "index.html").unlink()
+        built = self.build_page(scratch_builder)
+        assert built == ["index.html"]
+
+    def test_racy_source_changes(self, scratch_builder, monkeypatch):
+        # Modify source image immediately after the first call to Image.thumbnail().
+        # Note that this occurs *before* the thumbnail artifact is built.
+        #
+        # We are doing this to ensure there are no race conditions in
+        # the dependency tracking. Since the source was changed after the thumbnail
+        # parameters were computed, the artifact should be updated at the next build.
+
+        def thumbnail_advice(self, *args, **kwargs):
+            monkeypatch.undo()
+            try:
+                return Image.thumbnail(self, *args, **kwargs)
+            finally:
+                with open(self.attachment_filename, "ab") as fp:
+                    fp.write(b"\0")
+
+        monkeypatch.setattr(Image, "thumbnail", thumbnail_advice)
+
+        built = self.build_page(scratch_builder)
+        assert built == ["index.html", "test@20x20.jpg"]
+
+        Path(scratch_builder.destination_path, "index.html").unlink()
+        built = self.build_page(scratch_builder)
+        assert built == ["index.html", "test@20x20.jpg"]
+
+
 @dataclasses.dataclass
 class DemoThumbnail:
     width: int

tests/test_urls.py

@@ -2,7 +2,6 @@
 
 import pytest
 
-from lektor.context import _ctx_stack
 from lektor.context import Context
 from lektor.environment import Environment
 from lektor.reporter import BufferReporter
@@ -128,16 +127,14 @@ def test_url_to_page_with_explicit_alt(pad, alt, expected):
 
 
 @pytest.fixture
-def mock_build_context(mocker, pad):
-    ctx = mocker.Mock(spec=Context, pad=pad)
-    _ctx_stack.push(ctx)
-    try:
+def build_context(builder):
+    build_state = builder.new_build_state()
+    with Context(build_state.new_artifact("dummy-artifact")) as ctx:
         yield ctx
-    finally:
-        _ctx_stack.pop()
 
 
-def test_url_to_thumbnail(pad, mock_build_context):
+@pytest.mark.usefixtures("build_context")
+def test_url_to_thumbnail(pad):
     extra_de = pad.get("/extra", alt="de")
     thumbnail = pad.get("/test.jpg").thumbnail(42)
     assert extra_de.url_to(thumbnail) == "../../test@42x56.jpg"