diff --git a/photomap/backend/embeddings.py b/photomap/backend/embeddings.py index 37cf6cf3..a1984af3 100644 --- a/photomap/backend/embeddings.py +++ b/photomap/backend/embeddings.py @@ -41,7 +41,7 @@ capture_download_progress, get_cached_encoder, ) -from .media_types import IMAGE_EXTENSIONS +from .media_types import IMAGE_EXTENSIONS, is_video from .metadata_extraction import MetadataExtractor from .metadata_formatting import format_metadata from .metadata_modules import SlideSummary @@ -1857,6 +1857,18 @@ def find_duplicate_clusters(self, similarity_threshold=0.995): embeddings = data["embeddings"] filenames = data["filenames"] + # Videos are excluded from duplicate detection. Their embedding + # describes one extracted frame, and opening frames are frequently a + # black slate or a title card — two unrelated clips would then sit at + # ~1.0 cosine and be reported as duplicates, which users act on by + # deleting. Their frames can also legitimately duplicate a photo. + keep = np.array([not is_video(Path(str(f))) for f in filenames], dtype=bool) + if not keep.all(): + embeddings = embeddings[keep] + filenames = filenames[keep] + if len(embeddings) == 0: + return + # Normalize embeddings. ``_l2_normalize`` carries an epsilon guard so # an all-zero row can't produce NaN here. norm_embeddings = _l2_normalize(embeddings, axis=-1) diff --git a/photomap/backend/progress.py b/photomap/backend/progress.py index 38171f0f..fcf57874 100644 --- a/photomap/backend/progress.py +++ b/photomap/backend/progress.py @@ -85,8 +85,10 @@ def __init__(self): # folded into the ProgressInfo when the run completes (see # ``complete_operation``). Kept separate from ``_progress`` because the # per-phase ``start_operation`` calls recreate ProgressInfo and would - # otherwise wipe a warning recorded earlier in the same run. - self._completion_warnings: dict[str, str] = {} + # otherwise wipe a warning recorded earlier in the same run. Held as a + # list per album so several notices can accumulate and be compared + # exactly; joined with a space when handed to the poller. + self._completion_warnings: dict[str, list[str]] = {} self._lock = threading.Lock() def start_operation(self, album_key: str, total_images: int, operation_type: str): @@ -199,13 +201,36 @@ def set_completion_warning(self, album_key: str, message: str | None) -> None: and is folded in atomically by ``complete_operation``. A falsy ``message`` clears any pending notice so a clean re-run doesn't inherit a stale one. + + Replaces whatever was pending. Use :meth:`add_completion_warning` to + contribute an additional notice without discarding an existing one. """ with self._lock: if message: - self._completion_warnings[album_key] = message + self._completion_warnings[album_key] = [message] else: self._completion_warnings.pop(album_key, None) + def add_completion_warning(self, album_key: str, message: str | None) -> None: + """Append a notice, keeping any already pending for this album. + + A run can now produce more than one: a board album may have images + missing on disk *and* videos that could not be decoded. This used to + be a single slot, so the second writer silently discarded the first. + + Repeating an identical notice is ignored. Notices are held as a list so + that comparison is against whole notices: testing ``message not in + existing`` against the joined text made "2 videos could not be read." + a substring of "12 videos could not be read." and dropped it, which is + exactly the silent discard this method exists to prevent. + """ + if not message: + return + with self._lock: + pending = self._completion_warnings.setdefault(album_key, []) + if message not in pending: + pending.append(message) + def get_progress(self, album_key: str) -> ProgressInfo | None: """Get progress info for an album.""" with self._lock: @@ -258,9 +283,8 @@ def complete_operation( progress.images_processed = progress.total_images # Fold in (and consume) any pending non-fatal notice so it # lands atomically with the COMPLETED status the poller reads. - progress.warning_message = self._completion_warnings.pop( - album_key, None - ) + pending = self._completion_warnings.pop(album_key, None) + progress.warning_message = " ".join(pending) if pending else None # Global instance diff --git a/photomap/backend/routers/index.py b/photomap/backend/routers/index.py index 71f5b417..fd5bcc72 100644 --- a/photomap/backend/routers/index.py +++ b/photomap/backend/routers/index.py @@ -18,6 +18,7 @@ from .. import invokeai_client from ..config import get_config_manager from ..embeddings import LAST_UPDATED_FILENAME, Embeddings, peek_encoder_spec +from ..media_types import is_video from ..progress import IndexingCancelled, progress_tracker from .album import ( AlbumDep, @@ -69,6 +70,12 @@ class EmbeddingsIndexMetadata(BaseModel): filename_count: int embeddings_path: str last_modified: float + # Broken out so the album card can say "120 images, 4 videos" rather than + # leaving the user to wonder why the single count jumped. Derived from the + # filename suffixes, so indexes written before video support report + # image_count == filename_count and video_count == 0. + image_count: int = 0 + video_count: int = 0 # Note: How check_album_lock is used in this file: @@ -275,12 +282,16 @@ async def index_metadata(album_config: AlbumDep) -> EmbeddingsIndexMetadata: marker = index_path.parent / LAST_UPDATED_FILENAME if marker.exists(): last_modified = max(last_modified, marker.stat().st_mtime) - filename_count = len(Embeddings.open_cached_embeddings(index_path)["filenames"]) + filenames = Embeddings.open_cached_embeddings(index_path)["filenames"] + filename_count = len(filenames) + video_count = sum(1 for f in filenames if is_video(Path(str(f)))) return EmbeddingsIndexMetadata( filename_count=filename_count, embeddings_path=str(index_path), last_modified=last_modified, + image_count=filename_count - video_count, + video_count=video_count, ) diff --git a/photomap/backend/routers/invoke.py b/photomap/backend/routers/invoke.py index 2e554b51..feeb1d30 100644 --- a/photomap/backend/routers/invoke.py +++ b/photomap/backend/routers/invoke.py @@ -42,6 +42,7 @@ _request_with_auth_fallback, _validate_invokeai_url, ) +from ..media_types import is_video from ..metadata_modules.invoke.invoke_metadata_view import InvokeMetadataView from ..metadata_modules.invokemetadata import GenerationMetadataAdapter from .album import get_embeddings_for_album, require_no_lock @@ -494,7 +495,18 @@ def _load_image_path(album_key: str, index: int) -> Path: filenames = indexes["sorted_filenames"] if index < 0 or index >= len(filenames): raise HTTPException(status_code=404, detail="Index out of range") - return Path(str(filenames[index])) + path = Path(str(filenames[index])) + # The single choke point for both /recall and /use_ref_image, which upload + # the file to InvokeAI as a reference image. InvokeAI's upload endpoint + # takes images, so handing it an .mp4 would surface as an opaque 502 + # carrying raw upstream text. The drawer already withholds the button for + # videos; this closes the API path behind it. + if is_video(path): + raise HTTPException( + status_code=400, + detail="InvokeAI actions are not available for video files.", + ) + return path def _build_recall_payload(raw_metadata: dict, include_seed: bool) -> dict: diff --git a/photomap/backend/routers/search.py b/photomap/backend/routers/search.py index e13d7b70..eab84526 100644 --- a/photomap/backend/routers/search.py +++ b/photomap/backend/routers/search.py @@ -47,6 +47,23 @@ _MAX_THUMB_SIZE = 2048 _MAX_THUMB_RADIUS = 512 +# ``download_images_zip`` builds its archive in memory. Videos make it easy to +# ask for far more than fits, so cap the total selection size. +_MAX_ZIP_BYTES = 2_000_000_000 + + +def _format_bytes(size: int) -> str: + """Human-readable size for the download-limit message. + + Falls back to MB below a gigabyte so a lowered ceiling doesn't render as + "over the 0 GB download limit". + """ + if size >= 1_000_000_000: + return f"{size / 1_000_000_000:.1f} GB" + if size >= 1_000_000: + return f"{size / 1_000_000:.0f} MB" + return f"{size} bytes" + # Response Models class SearchResult(BaseModel): @@ -104,8 +121,22 @@ async def search_with_text_and_image( try: # If image_data is provided, decode and save to temp file if req.image_data: - image_bytes = base64.b64decode(req.image_data.split(",")[-1]) - query_image_data = Image.open(BytesIO(image_bytes)) + # A query blob that isn't a still image — a video file dropped on + # the search panel, say — used to surface as an opaque 500 from + # deep inside PIL. The encoder only takes stills. + try: + image_bytes = base64.b64decode(req.image_data.split(",")[-1]) + query_image_data = Image.open(BytesIO(image_bytes)) + # ``open`` only reads the header; without an explicit load the + # decode failure would surface later, from inside the encoder, + # as the 500 this guard is meant to replace. + query_image_data.load() + except Exception as e: + logger.info(f"Rejected an unreadable search query image: {e}") + raise HTTPException( + status_code=400, + detail="The query image could not be read. Search by image needs a still image.", + ) from e logger.info( f"Search request: {req.min_search_score=}, {req.max_search_results=}" @@ -531,6 +562,31 @@ async def download_images_zip( """ Download multiple images as a ZIP file. """ + # The archive is assembled entirely in memory, which was fine for photos + # but is not for video: twenty bookmarked 200 MB clips would be several + # gigabytes resident. Refuse above a ceiling rather than exhausting the + # server. + # Applies the same access check as the loop below, so the total only counts + # files that would actually be written. Counting a rejected path could + # refuse a selection that zips to nothing. + total_bytes = 0 + for index in req.indices: + try: + candidate = embeddings.get_image_path(index) + if validate_image_access(album_config, candidate) and candidate.is_file(): + total_bytes += candidate.stat().st_size + except Exception: + continue + if total_bytes > _MAX_ZIP_BYTES: + raise HTTPException( + status_code=413, + detail=( + f"That selection is {_format_bytes(total_bytes)}, over the " + f"{_format_bytes(_MAX_ZIP_BYTES)} download limit. " + "Select fewer files, or copy them to a folder instead." + ), + ) + # Create ZIP file in memory zip_buffer = BytesIO() with zipfile.ZipFile(zip_buffer, "w", zipfile.ZIP_DEFLATED) as zip_file: @@ -543,8 +599,13 @@ async def download_images_zip( if not image_path.exists() or not image_path.is_file(): logger.warning(f"Image not found at index {index}") continue + # Video containers hold already-compressed streams, so + # deflating them burns CPU for no gain. Store them instead. + compression = ( + zipfile.ZIP_STORED if is_video(image_path) else zipfile.ZIP_DEFLATED + ) # Add file to ZIP with just the filename (not full path) - zip_file.write(image_path, image_path.name) + zip_file.write(image_path, image_path.name, compress_type=compression) except Exception as e: logger.warning(f"Error adding image at index {index} to ZIP: {e}") continue diff --git a/photomap/frontend/static/javascript/bookmarks.js b/photomap/frontend/static/javascript/bookmarks.js index 0baa8467..862607ef 100644 --- a/photomap/frontend/static/javascript/bookmarks.js +++ b/photomap/frontend/static/javascript/bookmarks.js @@ -509,8 +509,26 @@ class BookmarkManager { async downloadSingleImage(globalIndex) { const data = await fetchJson(`retrieve_image/${encodeURIComponent(state.album)}/${globalIndex}`); - const imageUrl = data.image_url; - const filename = data.filename || `image_${globalIndex}.jpg`; + const isVideo = data.media_type === "video"; + // For a video, download the playable file rather than its still frame. + const imageUrl = isVideo && data.video_url ? data.video_url : data.image_url; + // Derive the fallback extension from the real path — the old hardcoded + // .jpg would save a video under a name no player would open. + const fallbackExtension = data.filepath?.split(".").pop() || (isVideo ? "mp4" : "jpg"); + const filename = data.filename || `image_${globalIndex}.${fallbackExtension}`; + + if (isVideo) { + // Videos are far too large to buffer into a blob: a 200 MB clip would + // sit entirely in browser memory before the save dialog appeared. Point + // the download straight at the URL and let the browser stream it. + const a = document.createElement("a"); + a.href = imageUrl; + a.download = filename; + document.body.appendChild(a); + a.click(); + document.body.removeChild(a); + return; + } // Fetch the actual image (binary, not JSON — fetch directly) const imageResponse = await fetch(imageUrl); diff --git a/tests/backend/test_video_guards.py b/tests/backend/test_video_guards.py new file mode 100644 index 00000000..f8406eb0 --- /dev/null +++ b/tests/backend/test_video_guards.py @@ -0,0 +1,449 @@ +"""Guards for the paths that assume an album entry is a still image. + +Each of these is somewhere a video would otherwise misbehave once indexing +picks them up: duplicate detection would report unrelated clips as identical, +the InvokeAI actions would upload an .mp4 as a reference image, a video blob +posted to search would 500, and the in-memory ZIP would happily try to hold +several gigabytes. +""" + +from __future__ import annotations + +import base64 +import shutil +import zipfile +from io import BytesIO +from pathlib import Path + +import numpy as np +import pytest +from fixtures import media_fixture_path +from PIL import Image + +from photomap.backend.embeddings import Embeddings, _open_npz_file +from photomap.backend.progress import ProgressTracker +from photomap.backend.util import atomic_savez + +ENCODER_SPEC = "openai-clip:ViT-B/32" +EMBEDDING_DIM = 8 + + +def _write_index(index_path: Path, files: list[Path], embeddings: np.ndarray) -> None: + index_path.parent.mkdir(parents=True, exist_ok=True) + atomic_savez( + index_path, + embeddings=embeddings.astype(np.float32), + filenames=np.array([f.resolve().as_posix() for f in files]), + modification_times=np.array( + [float(i + 1) for i in range(len(files))], dtype=float + ), + metadata=np.array([{} for _ in files], dtype=object), + model_id=np.array(ENCODER_SPEC), + embedding_dim=np.array(EMBEDDING_DIM), + ) + _open_npz_file.cache_clear() + + +# -------------------------------------------------------------------------- +# Duplicate detection +# -------------------------------------------------------------------------- + + +def test_duplicate_detection_ignores_videos(tmp_path, capsys): + """Two clips whose opening frames match must not be called duplicates. + + Opening frames are so often a black slate or a title card that unrelated + videos land at ~1.0 cosine — and users act on a duplicate report by + deleting. + """ + media = tmp_path / "media" + media.mkdir() + files = [media / "a.mp4", media / "b.mp4", media / "x.jpg", media / "y.jpg"] + for f in files: + f.write_bytes(b"stub") + + # Both pairs are internally identical. + identical = np.array([[1.0, 0.0] + [0.0] * 6]) + embeddings = np.vstack([identical, identical, identical, identical]) + + index_path = media / "index" / "embeddings.npz" + _write_index(index_path, files, embeddings) + + Embeddings(embeddings_path=index_path, encoder_spec=ENCODER_SPEC).find_duplicate_clusters() + + printed = capsys.readouterr().out + assert "x.jpg" in printed and "y.jpg" in printed + assert "a.mp4" not in printed and "b.mp4" not in printed + + +def test_duplicate_detection_on_an_all_video_index_is_a_noop(tmp_path, capsys): + media = tmp_path / "media" + media.mkdir() + files = [media / "a.mp4", media / "b.mp4"] + for f in files: + f.write_bytes(b"stub") + embeddings = np.vstack([np.array([[1.0] + [0.0] * 7])] * 2) + + index_path = media / "index" / "embeddings.npz" + _write_index(index_path, files, embeddings) + + Embeddings(embeddings_path=index_path, encoder_spec=ENCODER_SPEC).find_duplicate_clusters() + + assert capsys.readouterr().out.strip() == "" + + +# -------------------------------------------------------------------------- +# Search by image +# -------------------------------------------------------------------------- + + +def test_search_rejects_a_non_image_query_blob(client, new_album): + """A video dropped on the search panel gets a 400, not an opaque 500.""" + payload = base64.b64encode(media_fixture_path("clip.mp4").read_bytes()).decode() + + response = client.post( + f"/search_with_text_and_image/{new_album['key']}", + json={ + "image_data": f"data:video/mp4;base64,{payload}", + "positive_query": "", + "negative_query": "", + }, + ) + + assert response.status_code == 400 + assert "still image" in response.json()["detail"].lower() + + +def test_search_still_accepts_an_image_query(client, new_album): + """The guard must not cost us search-by-image. + + This has to build the index and assert a 200: without one the endpoint + 500s on the missing .npz, and an ``!= 400`` assertion passes on that 500 — + so the test would hold even if the guard rejected every valid image. + """ + from fixtures import build_index + + build_index(client, new_album) + + buffer = BytesIO() + Image.new("RGB", (64, 64), (10, 20, 30)).save(buffer, format="JPEG") + payload = base64.b64encode(buffer.getvalue()).decode() + + response = client.post( + f"/search_with_text_and_image/{new_album['key']}", + json={ + "image_data": f"data:image/jpeg;base64,{payload}", + "positive_query": "", + "negative_query": "", + }, + ) + + assert response.status_code == 200, response.text + assert len(response.json()) > 0 + + +# -------------------------------------------------------------------------- +# InvokeAI actions +# -------------------------------------------------------------------------- + + +@pytest.fixture +def video_album(client, tmp_path): + media = tmp_path / "invoke_media" + media.mkdir() + video = media / "clip.mp4" + shutil.copy(media_fixture_path("clip.mp4"), video) + + index_path = media / "index" / "embeddings.npz" + _write_index(index_path, [video], np.array([[1.0] + [0.0] * 7])) + + album = { + "key": "invoke_video_album", + "name": "Invoke Video Album", + "image_paths": [media.as_posix()], + "index": index_path.as_posix(), + "umap_eps": 0.1, + "description": "", + "encoder_spec": ENCODER_SPEC, + } + assert client.post("/add_album/", json=album).status_code == 201 + try: + yield album + finally: + client.delete(f"/delete_album/{album['key']}") + + +@pytest.fixture +def invokeai_configured(): + """Point the config at an InvokeAI URL that is never actually reached. + + The endpoints refuse early without a configured URL, which would mask the + guard under test. + """ + from photomap.backend.config import get_config_manager + + manager = get_config_manager() + manager.set_invokeai_settings(url="http://localhost:9090") + yield + manager.set_invokeai_settings(url=None, username=None, password=None) + + +def test_use_ref_image_refuses_videos(client, video_album, invokeai_configured): + """``_load_image_path`` is the choke point for the InvokeAI upload path. + + This endpoint POSTs the file to InvokeAI's /images/upload, which takes + images — an .mp4 would surface as an opaque 502 carrying raw upstream + text. The guard has to fire before any network call is attempted, which + is what makes this test safe to run with an unreachable URL configured. + """ + response = client.post( + "/invokeai/use_ref_image", + json={"album_key": video_album["key"], "index": 0}, + ) + assert response.status_code == 400 + assert "video" in response.json()["detail"].lower() + + +def test_recall_also_refuses_videos(client, video_album, invokeai_configured): + """Recall reaches videos through the metadata path rather than the file. + + It therefore already refused them — a video carries no InvokeAI + generation parameters — but the behaviour is worth pinning so a future + refactor that routes recall through the file path stays correct. + """ + response = client.post( + "/invokeai/recall", + json={"album_key": video_album["key"], "index": 0}, + ) + assert response.status_code == 400 + + +# -------------------------------------------------------------------------- +# ZIP download +# -------------------------------------------------------------------------- + + +def test_zip_download_refuses_an_oversized_selection( + client, new_album, monkeypatch +): + """The archive is built in memory, so the ceiling has to be enforced.""" + from photomap.backend.routers import search as search_router_module + + monkeypatch.setattr(search_router_module, "_MAX_ZIP_BYTES", 10) + + from fixtures import build_index + + build_index(client, new_album) + + response = client.post( + f"/download_images_zip/{new_album['key']}", json={"indices": [0, 1]} + ) + + assert response.status_code == 413 + assert "limit" in response.json()["detail"].lower() + + +def test_zip_stores_videos_and_deflates_images(client, tmp_path): + """Video streams are already compressed; deflating them burns CPU for ~0. + + Asserted on the archive that comes back rather than on the call, because + the compress_type is a per-member argument that is easy to drop without + any other visible effect. + """ + media = tmp_path / "zipmix" + media.mkdir() + video = media / "clip.mp4" + shutil.copy(media_fixture_path("clip.mp4"), video) + photo = media / "shot.png" + Image.new("RGB", (64, 64), (200, 40, 40)).save(photo) + + files = sorted([video, photo]) + index_path = media / "index" / "embeddings.npz" + _write_index( + index_path, files, np.vstack([np.array([[1.0] + [0.0] * 7])] * len(files)) + ) + + album = { + "key": "zipmix_album", + "name": "Zip Mix", + "image_paths": [media.as_posix()], + "index": index_path.as_posix(), + "umap_eps": 0.1, + "description": "", + "encoder_spec": ENCODER_SPEC, + } + assert client.post("/add_album/", json=album).status_code == 201 + try: + response = client.post( + "/download_images_zip/zipmix_album", json={"indices": [0, 1]} + ) + assert response.status_code == 200, response.text + + with zipfile.ZipFile(BytesIO(response.content)) as archive: + by_name = {info.filename: info for info in archive.infolist()} + assert by_name["clip.mp4"].compress_type == zipfile.ZIP_STORED + assert by_name["shot.png"].compress_type == zipfile.ZIP_DEFLATED + finally: + client.delete("/delete_album/zipmix_album") + + +def test_zip_size_check_ignores_files_it_would_not_include(client, new_album, monkeypatch): + """The ceiling must be measured over what actually gets written. + + An index can name a path outside the album (a moved directory, an edited + config); those are skipped when building the archive, so counting their + bytes could refuse a selection that zips to almost nothing. + """ + from fixtures import build_index + + from photomap.backend.routers import search as search_router_module + + build_index(client, new_album) + + monkeypatch.setattr( + search_router_module, "validate_image_access", lambda *args, **kwargs: False + ) + monkeypatch.setattr(search_router_module, "_MAX_ZIP_BYTES", 1) + + response = client.post( + f"/download_images_zip/{new_album['key']}", json={"indices": [0, 1]} + ) + + # Nothing is includable, so nothing counts: an empty archive, not a 413. + assert response.status_code == 200, response.text + with zipfile.ZipFile(BytesIO(response.content)) as archive: + assert archive.namelist() == [] + + +def test_zip_download_still_works_under_the_limit(client, new_album): + from fixtures import build_index + + build_index(client, new_album) + + response = client.post( + f"/download_images_zip/{new_album['key']}", json={"indices": [0]} + ) + + assert response.status_code == 200 + assert response.headers["content-type"] == "application/zip" + + +# -------------------------------------------------------------------------- +# Index metadata +# -------------------------------------------------------------------------- + + +def test_index_metadata_splits_images_and_videos(client, tmp_path): + media = tmp_path / "counted" + media.mkdir() + files = [media / "a.mp4", media / "b.jpg", media / "c.jpg"] + for f in files: + f.write_bytes(b"stub") + _write_index( + media / "index" / "embeddings.npz", + files, + np.vstack([np.array([[1.0] + [0.0] * 7])] * 3), + ) + + album = { + "key": "counted_album", + "name": "Counted", + "image_paths": [media.as_posix()], + "index": (media / "index" / "embeddings.npz").as_posix(), + "umap_eps": 0.1, + "description": "", + "encoder_spec": ENCODER_SPEC, + } + assert client.post("/add_album/", json=album).status_code == 201 + try: + payload = client.get("/index_metadata/counted_album").json() + assert payload["filename_count"] == 3 + assert payload["image_count"] == 2 + assert payload["video_count"] == 1 + finally: + client.delete("/delete_album/counted_album") + + +def test_index_metadata_reports_legacy_indexes_as_all_images(client, new_album): + from fixtures import build_index + + build_index(client, new_album) + payload = client.get(f"/index_metadata/{new_album['key']}").json() + assert payload["video_count"] == 0 + assert payload["image_count"] == payload["filename_count"] + + +# -------------------------------------------------------------------------- +# Completion warnings +# -------------------------------------------------------------------------- + + +def test_completion_warnings_compose(): + """A run can now produce more than one non-fatal notice. + + A board album may have images missing on disk *and* videos that could not + be decoded; the single slot used to let the second writer discard the + first silently. + """ + tracker = ProgressTracker() + tracker.set_completion_warning("album", "3 images missing on disk.") + tracker.add_completion_warning("album", "2 videos could not be read.") + + tracker.start_operation("album", total_images=1, operation_type="indexing") + tracker.complete_operation("album") + + warning = tracker.get_progress("album").warning_message + assert "3 images missing on disk." in warning + assert "2 videos could not be read." in warning + + +def test_adding_the_same_warning_twice_does_not_duplicate_it(): + tracker = ProgressTracker() + tracker.add_completion_warning("album", "2 videos could not be read.") + tracker.add_completion_warning("album", "2 videos could not be read.") + + tracker.start_operation("album", total_images=1, operation_type="indexing") + tracker.complete_operation("album") + + assert tracker.get_progress("album").warning_message.count("2 videos") == 1 + + +def test_adding_a_warning_that_is_a_substring_of_another_keeps_both(): + """Deduplication compares whole notices, not substrings of the joined text. + + "2 videos could not be read." is a substring of "12 videos could not be + read.", so a containment check against the accumulated string dropped it — + the exact silent discard this method exists to prevent. + """ + tracker = ProgressTracker() + tracker.add_completion_warning("album", "12 videos could not be read.") + tracker.add_completion_warning("album", "2 videos could not be read.") + + tracker.start_operation("album", total_images=1, operation_type="indexing") + tracker.complete_operation("album") + + warning = tracker.get_progress("album").warning_message + assert "12 videos could not be read." in warning + assert warning.count("videos could not be read.") == 2 + + +def test_adding_an_empty_warning_is_a_noop(): + tracker = ProgressTracker() + tracker.add_completion_warning("album", "kept") + tracker.add_completion_warning("album", None) + + tracker.start_operation("album", total_images=1, operation_type="indexing") + tracker.complete_operation("album") + + assert tracker.get_progress("album").warning_message == "kept" + + +def test_set_completion_warning_still_replaces(): + tracker = ProgressTracker() + tracker.set_completion_warning("album", "first") + tracker.set_completion_warning("album", "second") + + tracker.start_operation("album", total_images=1, operation_type="indexing") + tracker.complete_operation("album") + + assert tracker.get_progress("album").warning_message == "second" diff --git a/tests/frontend/bookmarks-download.test.js b/tests/frontend/bookmarks-download.test.js new file mode 100644 index 00000000..57128c99 --- /dev/null +++ b/tests/frontend/bookmarks-download.test.js @@ -0,0 +1,155 @@ +// downloadSingleImage has to treat a video differently from a photo: a photo +// is fetched into a blob so the object URL can carry a chosen filename, but a +// 200 MB clip buffered the same way sits entirely in browser memory before the +// save dialog appears. +import { beforeEach, describe, expect, it, jest } from "@jest/globals"; + +const mockFetchJson = jest.fn(); + +jest.unstable_mockModule("../../photomap/frontend/static/javascript/utils.js", () => ({ + errorDetail: jest.fn(), + fetchJson: mockFetchJson, + hideSpinner: jest.fn(), + setCheckmarkOnIcon: jest.fn(), + showSpinner: jest.fn(), +})); +jest.unstable_mockModule("../../photomap/frontend/static/javascript/control-panel.js", () => ({ + showDeleteConfirmModal: jest.fn(), +})); +jest.unstable_mockModule("../../photomap/frontend/static/javascript/filetree.js", () => ({ + createSimpleDirectoryPicker: jest.fn(), +})); +jest.unstable_mockModule("../../photomap/frontend/static/javascript/index.js", () => ({ + deleteImages: jest.fn(), +})); +jest.unstable_mockModule("../../photomap/frontend/static/javascript/modal-utils.js", () => ({ + showConfirmModal: jest.fn(), +})); +jest.unstable_mockModule("../../photomap/frontend/static/javascript/search.js", () => ({ + setSearchResults: jest.fn(), +})); +jest.unstable_mockModule("../../photomap/frontend/static/javascript/slide-state.js", () => ({ + slideState: { getCurrentSlide: () => ({ globalIndex: 0 }) }, +})); +jest.unstable_mockModule("../../photomap/frontend/static/javascript/state.js", () => ({ + state: { album: "album", swiper: null, single_swiper: null }, + saveSettingsToLocalStorage: jest.fn(), +})); + +const { bookmarkManager } = await import("../../photomap/frontend/static/javascript/bookmarks.js"); + +/** Capture the the download path synthesises, and swallow its click. */ +function captureAnchor() { + const anchors = []; + const realCreate = document.createElement.bind(document); + jest.spyOn(document, "createElement").mockImplementation((tag) => { + const el = realCreate(tag); + if (tag === "a") { + el.click = jest.fn(); + anchors.push(el); + } + return el; + }); + return anchors; +} + +beforeEach(() => { + jest.restoreAllMocks(); + mockFetchJson.mockReset(); + global.fetch = jest.fn(); + global.URL.createObjectURL = jest.fn(() => "blob:mock"); + global.URL.revokeObjectURL = jest.fn(); +}); + +describe("downloading a video", () => { + const VIDEO = { + media_type: "video", + image_url: "video_frame/album/3", + video_url: "videos/album/clip.mp4", + filename: "clip.mp4", + filepath: "/photos/clip.mp4", + }; + + it("links straight at the video instead of buffering it into a blob", async () => { + mockFetchJson.mockResolvedValue(VIDEO); + const anchors = captureAnchor(); + + await bookmarkManager.downloadSingleImage(3); + + // No fetch of the media itself, and no object URL: that is the whole point. + expect(global.fetch).not.toHaveBeenCalled(); + expect(global.URL.createObjectURL).not.toHaveBeenCalled(); + + expect(anchors).toHaveLength(1); + expect(anchors[0].getAttribute("href")).toBe("videos/album/clip.mp4"); + expect(anchors[0].download).toBe("clip.mp4"); + expect(anchors[0].click).toHaveBeenCalled(); + }); + + it("downloads the playable file, not the still frame", async () => { + mockFetchJson.mockResolvedValue(VIDEO); + const anchors = captureAnchor(); + + await bookmarkManager.downloadSingleImage(3); + + expect(anchors[0].getAttribute("href")).not.toBe(VIDEO.image_url); + }); + + it("falls back to the still frame when there is no video URL", async () => { + mockFetchJson.mockResolvedValue({ ...VIDEO, video_url: "" }); + const anchors = captureAnchor(); + + await bookmarkManager.downloadSingleImage(3); + + expect(anchors[0].getAttribute("href")).toBe("video_frame/album/3"); + }); + + it("names an unnamed video with its real extension, not .jpg", async () => { + // The old hardcoded image_${i}.jpg would save a clip under a name no + // player would open. + mockFetchJson.mockResolvedValue({ ...VIDEO, filename: "", filepath: "/photos/holiday.webm" }); + const anchors = captureAnchor(); + + await bookmarkManager.downloadSingleImage(7); + + expect(anchors[0].download).toBe("image_7.webm"); + }); + + it("still names a video when the index carries no path at all", async () => { + mockFetchJson.mockResolvedValue({ ...VIDEO, filename: "", filepath: undefined }); + const anchors = captureAnchor(); + + await bookmarkManager.downloadSingleImage(7); + + expect(anchors[0].download).toBe("image_7.mp4"); + }); +}); + +describe("downloading a still image", () => { + const IMAGE = { + media_type: "image", + image_url: "images/album/shot.jpg", + filename: "shot.jpg", + filepath: "/photos/shot.jpg", + }; + + it("keeps the blob path, so the chosen filename is honoured", async () => { + mockFetchJson.mockResolvedValue(IMAGE); + global.fetch.mockResolvedValue({ ok: true, blob: async () => new Blob(["x"]) }); + const anchors = captureAnchor(); + + await bookmarkManager.downloadSingleImage(1); + + expect(global.fetch).toHaveBeenCalledWith("images/album/shot.jpg"); + expect(global.URL.createObjectURL).toHaveBeenCalled(); + expect(anchors[0].getAttribute("href")).toBe("blob:mock"); + expect(anchors[0].download).toBe("shot.jpg"); + }); + + it("still throws when the image cannot be fetched", async () => { + mockFetchJson.mockResolvedValue(IMAGE); + global.fetch.mockResolvedValue({ ok: false }); + + await expect(bookmarkManager.downloadSingleImage(1)).rejects.toThrow(/Failed to fetch/); + }); +});