diff --git a/faststack/app.py b/faststack/app.py index 37efddc..1f77d70 100644 --- a/faststack/app.py +++ b/faststack/app.py @@ -357,6 +357,7 @@ def __init__( get_metadata_callback=self._get_metadata_dict, get_batch_indices_callback=self._get_batch_indices, get_current_index_callback=self._get_current_loupe_index, + metadata_key_fn=lambda p: self.sidecar.metadata_key_for_path(p), thumbnail_size=200, parent=self, # Ensure proper Qt ownership to prevent GC issues ) @@ -1040,13 +1041,9 @@ def _apply_filter_to_cached_list(self): # Apply flag-based filtering (AND logic: image must have ALL checked flags) if self._filter_enabled and self._filter_flags: flags = self._filter_flags - # Optimize: access sidecar entries directly to avoid get_metadata overhead - entries = self.sidecar.data.entries result = [] for img in filtered: - # Direct dict lookup is faster than get_metadata() which might create objects - stem = img.path.stem - meta = entries.get(stem) + meta = self.sidecar.get_metadata(img.path, create=False) if not meta: continue @@ -1810,7 +1807,7 @@ def jump_to_last_uploaded(self): for idx in range(len(self.image_files) - 1, -1, -1): img = self.image_files[idx] # Dynamic look-up of self.sidecar as requested (important for mocks in tests) - meta = self.sidecar.get_metadata(img.path.stem, create=False) + meta = self.sidecar.get_metadata(img.path, create=False) uploaded = meta.uploaded if meta else False @@ -2126,10 +2123,10 @@ def _on_thumbnail_ready_gui(self, thumbnail_id: str): if self._thumbnail_model: self._thumbnail_model.thumbnailReady.emit(thumbnail_id) - def _get_metadata_dict(self, stem: str) -> dict: - """Get metadata for a file stem as a dict for thumbnail model.""" + def _get_metadata_dict(self, image_path: Path | str) -> dict: + """Get metadata for an image path as a dict for thumbnail model.""" try: - meta = self.sidecar.get_metadata(stem, create=False) + meta = self.sidecar.get_metadata(image_path, create=False) if meta is None: return { "stacked": False, @@ -2148,7 +2145,7 @@ def _get_metadata_dict(self, stem: str) -> dict: "todo": meta.todo, } except Exception as e: # Broad catch for UI plumbing - don't crash grid view - log.debug("Failed to get metadata for %s: %s", stem, e) + log.debug("Failed to get metadata for %s: %s", image_path, e) return { "stacked": False, "uploaded": False, @@ -2162,9 +2159,12 @@ def _get_bulk_metadata_map(self) -> Dict[str, dict]: """Get flattened metadata map for all images (for efficient grid refresh).""" bulk_map = {} try: - # sidecar.data.entries is a dict of stem -> EntryMetadata - for stem, meta in self.sidecar.data.entries.items(): - bulk_map[stem] = { + for img in self.image_files: + key = self.sidecar.metadata_key_for_path(img.path) + meta = self.sidecar.get_metadata(img.path, create=False) + if meta is None: + continue + bulk_map[key] = { "stacked": getattr(meta, "stacked", False), "uploaded": getattr(meta, "uploaded", False), "edited": getattr(meta, "edited", False), @@ -2214,8 +2214,8 @@ def toggle_uploaded(self): return today = datetime.now().strftime("%Y-%m-%d") - stem = self.image_files[self.current_index].path.stem - meta = self.sidecar.get_metadata(stem) + image_path = self.image_files[self.current_index].path + meta = self.sidecar.get_metadata(image_path) meta.uploaded = not meta.uploaded if meta.uploaded: @@ -2229,7 +2229,7 @@ def toggle_uploaded(self): self.sync_ui_state() status = "uploaded" if meta.uploaded else "not uploaded" self.update_status_message(f"Marked as {status}") - log.info("Toggled uploaded flag to %s for %s", meta.uploaded, stem) + log.info("Toggled uploaded flag to %s for %s", meta.uploaded, image_path) def toggle_todo(self): """Toggle todo flag for current image.""" @@ -2237,8 +2237,8 @@ def toggle_todo(self): return today = datetime.now().strftime("%Y-%m-%d") - stem = self.image_files[self.current_index].path.stem - meta = self.sidecar.get_metadata(stem) + image_path = self.image_files[self.current_index].path + meta = self.sidecar.get_metadata(image_path) meta.todo = not getattr(meta, "todo", False) if meta.todo: @@ -2252,7 +2252,7 @@ def toggle_todo(self): self.sync_ui_state() status = "todo" if meta.todo else "not todo" self.update_status_message(f"Marked as {status}") - log.info("Toggled todo flag to %s for %s", meta.todo, stem) + log.info("Toggled todo flag to %s for %s", meta.todo, image_path) def toggle_edited(self): """Toggle edited flag for current image.""" @@ -2260,8 +2260,8 @@ def toggle_edited(self): return today = datetime.now().strftime("%Y-%m-%d") - stem = self.image_files[self.current_index].path.stem - meta = self.sidecar.get_metadata(stem) + image_path = self.image_files[self.current_index].path + meta = self.sidecar.get_metadata(image_path) meta.edited = not meta.edited if meta.edited: @@ -2275,7 +2275,7 @@ def toggle_edited(self): self.sync_ui_state() status = "edited" if meta.edited else "not edited" self.update_status_message(f"Marked as {status}") - log.info("Toggled edited flag to %s for %s", meta.edited, stem) + log.info("Toggled edited flag to %s for %s", meta.edited, image_path) def toggle_restacked(self): """Toggle restacked flag for current image.""" @@ -2283,8 +2283,8 @@ def toggle_restacked(self): return today = datetime.now().strftime("%Y-%m-%d") - stem = self.image_files[self.current_index].path.stem - meta = self.sidecar.get_metadata(stem) + image_path = self.image_files[self.current_index].path + meta = self.sidecar.get_metadata(image_path) meta.restacked = not meta.restacked if meta.restacked: @@ -2298,15 +2298,15 @@ def toggle_restacked(self): self.sync_ui_state() status = "restacked" if meta.restacked else "not restacked" self.update_status_message(f"Marked as {status}") - log.info("Toggled restacked flag to %s for %s", meta.restacked, stem) + log.info("Toggled restacked flag to %s for %s", meta.restacked, image_path) def toggle_favorite(self): """Toggle favorite flag for current image.""" if not self.image_files or self.current_index >= len(self.image_files): return - stem = self.image_files[self.current_index].path.stem - meta = self.sidecar.get_metadata(stem) + image_path = self.image_files[self.current_index].path + meta = self.sidecar.get_metadata(image_path) meta.favorite = not meta.favorite @@ -2316,7 +2316,7 @@ def toggle_favorite(self): self.sync_ui_state() status = "Favorited" if meta.favorite else "Unfavorited" self.update_status_message(status) - log.info("Toggled favorite flag to %s for %s", meta.favorite, stem) + log.info("Toggled favorite flag to %s for %s", meta.favorite, image_path) def toggle_stacked(self): """Toggle stacked flag for current image.""" @@ -2324,8 +2324,8 @@ def toggle_stacked(self): return today = datetime.now().strftime("%Y-%m-%d") - stem = self.image_files[self.current_index].path.stem - meta = self.sidecar.get_metadata(stem) + image_path = self.image_files[self.current_index].path + meta = self.sidecar.get_metadata(image_path) meta.stacked = not meta.stacked if meta.stacked: @@ -2339,7 +2339,7 @@ def toggle_stacked(self): self.sync_ui_state() status = "stacked" if meta.stacked else "not stacked" self.update_status_message(f"Marked as {status}") - log.info("Toggled stacked flag to %s for %s", meta.stacked, stem) + log.info("Toggled stacked flag to %s for %s", meta.stacked, image_path) def get_current_metadata(self) -> Dict: if not self.image_files or self.current_index >= len(self.image_files): @@ -2357,8 +2357,8 @@ def get_current_metadata(self) -> Dict: return self._metadata_cache # Compute and cache - stem = self.image_files[self.current_index].path.stem - meta = self.sidecar.get_metadata(stem, create=False) + image_path = self.image_files[self.current_index].path + meta = self.sidecar.get_metadata(image_path, create=False) stack_info = self._get_stack_info(self.current_index) batch_info = self._get_batch_info(self.current_index) @@ -2619,7 +2619,7 @@ def add_favorites_to_batch(self): # Find indices of all favorited images indices_to_add = [] for i, img in enumerate(self.image_files): - meta = self.sidecar.get_metadata(img.path.stem, create=False) + meta = self.sidecar.get_metadata(img.path, create=False) if meta and meta.favorite: indices_to_add.append(i) @@ -2674,7 +2674,7 @@ def add_uploaded_to_batch(self): # Find indices of all uploaded images indices_to_add = [] for i, img in enumerate(self.image_files): - meta = self.sidecar.get_metadata(img.path.stem, create=False) + meta = self.sidecar.get_metadata(img.path, create=False) if meta and meta.uploaded: indices_to_add.append(i) @@ -3099,8 +3099,7 @@ def _launch_helicon_with_files(self, files: List[Path]) -> bool: for img_file in self.image_files: # Match by either RAW pair or JPG path if img_file.raw_pair == file_path or img_file.path == file_path: - stem = img_file.path.stem - meta = self.sidecar.get_metadata(stem) + meta = self.sidecar.get_metadata(img_file.path) meta.stacked = True meta.stacked_date = today break @@ -5448,8 +5447,7 @@ def edit_in_photoshop(self): # Mark as edited on successful launch today = datetime.now().strftime("%Y-%m-%d") - stem = image_file.path.stem - meta = self.sidecar.get_metadata(stem) + meta = self.sidecar.get_metadata(image_file.path) meta.edited = True meta.edited_date = today self.sidecar.save() @@ -5583,8 +5581,7 @@ def start_drag_current_image(self): today = datetime.now().strftime("%Y-%m-%d") for idx in existing_indices: - stem = self.image_files[idx].path.stem - meta = self.sidecar.get_metadata(stem) + meta = self.sidecar.get_metadata(self.image_files[idx].path) meta.uploaded = True meta.uploaded_date = today @@ -6456,8 +6453,7 @@ def stack_source_raws(self): if success: # Mark as restacked on success today = datetime.now().strftime("%Y-%m-%d") - stem = self.image_files[self.current_index].path.stem - meta = self.sidecar.get_metadata(stem) + meta = self.sidecar.get_metadata(self.image_files[self.current_index].path) meta.restacked = True meta.restacked_date = today self.sidecar.save() @@ -7363,8 +7359,9 @@ def get_stack_summary(self) -> str: def is_stacked(self) -> bool: if not self.image_files or self.current_index >= len(self.image_files): return False - stem = self.image_files[self.current_index].path.stem - meta = self.sidecar.get_metadata(stem, create=False) + meta = self.sidecar.get_metadata( + self.image_files[self.current_index].path, create=False + ) return meta.stacked if meta else False def _update_cache_stats(self): diff --git a/faststack/io/sidecar.py b/faststack/io/sidecar.py index c1adcf0..4daba69 100644 --- a/faststack/io/sidecar.py +++ b/faststack/io/sidecar.py @@ -2,13 +2,16 @@ import json import logging +import os import time from pathlib import Path -from typing import Literal, Optional, overload +from typing import Literal, Optional, Union, overload +from faststack.io.indexer import JPG_EXTENSIONS, RAW_EXTENSIONS from faststack.models import Sidecar, EntryMetadata log = logging.getLogger(__name__) +KNOWN_IMAGE_EXTENSIONS = frozenset(ext.lower() for ext in JPG_EXTENSIONS | RAW_EXTENSIONS) def _entrymetadata_from_json(meta: dict) -> EntryMetadata: @@ -37,6 +40,7 @@ def _entrymetadata_from_json(meta: dict) -> EntryMetadata: class SidecarManager: def __init__(self, directory: Path, watcher, debug: bool = False): + self.directory = directory self.path = directory / "faststack.json" self.watcher = watcher self.debug = debug @@ -71,8 +75,8 @@ def load(self) -> Sidecar: # Reconstruct nested objects entries = { - stem: _entrymetadata_from_json(meta) - for stem, meta in data.get("entries", {}).items() + key: _entrymetadata_from_json(meta) + for key, meta in data.get("entries", {}).items() } return Sidecar( version=data.get("version", 2), @@ -103,7 +107,7 @@ def save(self): "version": self.data.version, "last_index": self.data.last_index, "entries": { - stem: meta.__dict__ for stem, meta in self.data.entries.items() + key: meta.__dict__ for key, meta in self.data.entries.items() }, "stacks": self.data.stacks, } @@ -121,21 +125,21 @@ def save(self): @overload def get_metadata( - self, image_stem: str, *, create: Literal[True] = True + self, image_ref: Union[str, Path], *, create: Literal[True] = True ) -> EntryMetadata: ... @overload def get_metadata( - self, image_stem: str, *, create: Literal[False] + self, image_ref: Union[str, Path], *, create: Literal[False] ) -> Optional[EntryMetadata]: ... @overload def get_metadata( - self, image_stem: str, *, create: bool + self, image_ref: Union[str, Path], *, create: bool ) -> Optional[EntryMetadata]: ... def get_metadata( - self, image_stem: str, *, create: bool = True + self, image_ref: Union[str, Path], *, create: bool = True ) -> Optional[EntryMetadata]: """Get metadata for an image, optionally creating a persistent entry. @@ -143,11 +147,124 @@ def get_metadata( and storing one if it doesn't exist). When create=False, returns None if no entry exists — callers must handle the None case explicitly. """ - meta = self.data.entries.get(image_stem) + stable_key, candidate_keys = self._lookup_keys(image_ref) + if not stable_key: + if create: + raise ValueError(f"image_ref must not be empty: {image_ref!r}") + return None + + meta = self.data.entries.get(stable_key) + if meta is None: + for candidate_key in candidate_keys: + if candidate_key == stable_key: + continue + candidate_meta = self.data.entries.get(candidate_key) + if candidate_meta is None: + continue + meta = candidate_meta + if stable_key not in self.data.entries: + self.data.entries[stable_key] = candidate_meta + if candidate_key in self.data.entries and candidate_key != stable_key: + del self.data.entries[candidate_key] + break + if meta is None: + for existing_key, existing_meta in list(self.data.entries.items()): + if existing_key == stable_key: + continue + if self._stable_key_from_key(existing_key, check_fs=True) != stable_key: + continue + meta = existing_meta + self.data.entries[stable_key] = existing_meta + del self.data.entries[existing_key] + break + if meta is None and create: meta = EntryMetadata() - self.data.entries[image_stem] = meta + self.data.entries[stable_key] = meta return meta + def metadata_key_for_path(self, image_path: Union[str, Path]) -> str: + """Return the stable sidecar key for a concrete image path.""" + path = Path(image_path) + if not path.name: + return "" + if not path.is_absolute(): + path = self.directory / path + base_dir = Path(os.path.normcase(os.path.abspath(str(self.directory)))) + abs_path = Path(os.path.normcase(os.path.abspath(str(path)))) + + try: + relative = abs_path.relative_to(base_dir) + stable_path = relative.parent / relative.stem + return str(stable_path).replace("\\", "/") + except ValueError: + stable_path = abs_path.parent / abs_path.stem + return str(stable_path).replace("\\", "/") + + def _lookup_keys(self, image_ref: Union[str, Path]) -> tuple[str, list[str]]: + """Return (stable_key, migration_candidate_keys) for a metadata lookup.""" + if isinstance(image_ref, Path): + if not image_ref.name: + return "", [] + stable_key = self.metadata_key_for_path(image_ref) + full_name_key = self._metadata_filename_key(image_ref) + return stable_key, [full_name_key, image_ref.stem] + + value = str(image_ref) + if not value: + return "", [] + + # Only treat string as a path if it contains explicit path separators. + # Dotted strings (even with image extensions like "photo.CR2") are + # treated as exact keys — migration of legacy filename keys is handled + # by the _stable_key_from_key scan in get_metadata. + if os.path.sep in value or "/" in value or "\\" in value: + path = Path(value) + stable_key = self.metadata_key_for_path(path) + full_name_key = self._metadata_filename_key(path) + return stable_key, [full_name_key, path.stem] + + return value, [value] + + def _metadata_filename_key(self, image_path: Union[str, Path]) -> str: + """Return the extension-preserving key used by the regressed patch.""" + path = Path(image_path) + if not path.name: + return "" + if not path.is_absolute(): + path = self.directory / path + base_dir = Path(os.path.normcase(os.path.abspath(str(self.directory)))) + abs_path = Path(os.path.normcase(os.path.abspath(str(path)))) + + try: + relative = abs_path.relative_to(base_dir) + return str(relative).replace("\\", "/") + except ValueError: + return str(abs_path).replace("\\", "/") + + def _stable_key_from_key(self, key: str, check_fs: bool = False) -> str: + """Convert any historical sidecar key form into today's stable key. + + Args: + key: The sidecar key to normalize. + check_fs: If True, check the filesystem for bare-stem keys that + match an existing file. Set to True during one-time migration + scans; leave False on hot paths to avoid filesystem I/O. + """ + if not key: + return "" + if ( + os.path.sep in key + or "/" in key + or "\\" in key + or Path(key).suffix.lower() in KNOWN_IMAGE_EXTENSIONS + ): + return self.metadata_key_for_path(Path(key)) + if check_fs: + candidate_path = self.directory / key + if candidate_path.exists(): + return self.metadata_key_for_path(candidate_path) + return key + def set_last_index(self, index: int): self.data.last_index = index diff --git a/faststack/io/utils.py b/faststack/io/utils.py index 6c0d11f..e130440 100644 --- a/faststack/io/utils.py +++ b/faststack/io/utils.py @@ -20,7 +20,7 @@ def normalize_path_key(path: Union[Path, str]) -> str: # os.path.normcase on Linux: returns as-is # os.path.abspath: ensures absolute path and collapses .. # Note: abspath does NOT resolve symlinks (that's realpath). - return os.path.normcase(os.path.abspath(p_str)) + return os.path.normcase(os.path.abspath(p_str)).replace("\\", "/") def compute_path_hash(path: Union[Path, str]) -> str: diff --git a/faststack/logging_setup.py b/faststack/logging_setup.py index ab684a4..435010d 100644 --- a/faststack/logging_setup.py +++ b/faststack/logging_setup.py @@ -3,15 +3,71 @@ import logging import logging.handlers import os +import sys +import tempfile from pathlib import Path +log = logging.getLogger(__name__) + + +def _is_writable_dir(path: Path) -> bool: + """Return True when an existing directory accepts file writes.""" + if not path.is_dir(): + return False + + try: + with tempfile.NamedTemporaryFile( + mode="w", encoding="utf-8", dir=path, prefix="faststack-write-", delete=True + ) as f: + f.write("ok") + return True + except OSError: + return False + + +def _can_create_dir(path: Path) -> bool: + """Return True when the nearest existing parent is writable.""" + parent = path + while not parent.exists(): + next_parent = parent.parent + if next_parent == parent: + return False + parent = next_parent + + return _is_writable_dir(parent) + def get_app_data_dir() -> Path: - """Returns the application data directory.""" + """Return a writable application data directory, with fallbacks.""" + candidates = [] + app_data = os.getenv("APPDATA") if app_data: - return Path(app_data) / "faststack" - return Path.home() / ".faststack" + candidates.append(Path(app_data) / "faststack") + + local_app_data = os.getenv("LOCALAPPDATA") + if local_app_data: + candidates.append(Path(local_app_data) / "faststack") + + candidates.append(Path.home() / ".faststack") + candidates.append(Path.cwd() / "var" / "appdata") + + for candidate in candidates: + if _is_writable_dir(candidate): + return candidate + + for candidate in candidates: + if _can_create_dir(candidate): + return candidate + + # Final fallback: system temp is the most reliable writable location. + fallback = Path(tempfile.gettempdir()) / "faststack" + log.warning( + "No writable app-data directory found; falling back to temp directory %s. " + "Configuration and logs may not persist across restarts.", + fallback, + ) + return fallback def setup_logging(debug: bool = False): @@ -21,17 +77,27 @@ def setup_logging(debug: bool = False): debug: If True, sets log level to DEBUG. Otherwise, sets to WARNING to reduce noise. """ log_dir = get_app_data_dir() / "logs" - log_dir.mkdir(parents=True, exist_ok=True) - log_file = log_dir / "app.log" + log_file = None + try: + log_dir.mkdir(parents=True, exist_ok=True) + except OSError: + log_dir = Path(tempfile.gettempdir()) / "faststack" / "logs" + try: + log_dir.mkdir(parents=True, exist_ok=True) + except OSError: + log_dir = None + + if log_dir is None: + sys.stderr.write( + "WARNING: Could not create log directory; logs will not be persisted.\n" + ) + + if log_dir is not None: + log_file = log_dir / "app.log" - # File handler - file_handler = logging.handlers.RotatingFileHandler( - log_file, maxBytes=10 * 1024 * 1024, backupCount=5 - ) formatter = logging.Formatter( "%(asctime)s - %(name)s - %(levelname)s - %(message)s" ) - file_handler.setFormatter(formatter) # Console handler (for seeing logs in terminal) console_handler = logging.StreamHandler() @@ -41,9 +107,15 @@ def setup_logging(debug: bool = False): # Set log level based on debug flag root_logger.setLevel(logging.DEBUG if debug else logging.WARNING) root_logger.handlers.clear() - root_logger.addHandler(file_handler) root_logger.addHandler(console_handler) + if log_file is not None: + file_handler = logging.handlers.RotatingFileHandler( + log_file, maxBytes=10 * 1024 * 1024, backupCount=5 + ) + file_handler.setFormatter(formatter) + root_logger.addHandler(file_handler) + # Configure logging for key modules if debug: logging.getLogger("faststack.imaging.cache").setLevel(logging.DEBUG) diff --git a/faststack/tests/test_jump_to_last_uploaded.py b/faststack/tests/test_jump_to_last_uploaded.py index 524d8a3..80e21da 100644 --- a/faststack/tests/test_jump_to_last_uploaded.py +++ b/faststack/tests/test_jump_to_last_uploaded.py @@ -66,8 +66,12 @@ def test_jump_to_last_uploaded_success(mock_controller): meta2 = EntryMetadata(uploaded=False) meta3 = EntryMetadata(uploaded=True) - def side_effect(stem, **kwargs): - return {"img1": meta1, "img2": meta2, "img3": meta3}.get(stem, EntryMetadata()) + def side_effect(path, **kwargs): + return { + Path("img1.jpg"): meta1, + Path("img2.jpg"): meta2, + Path("img3.jpg"): meta3, + }.get(Path(path), EntryMetadata()) mock_controller.sidecar.get_metadata.side_effect = side_effect @@ -137,8 +141,12 @@ def test_jump_to_last_uploaded_one(mock_controller): mock_controller.image_files = [img1, img2, img3] mock_controller.current_index = 0 - def side_effect(stem, **kwargs): - return {"img1": meta1, "img2": meta2, "img3": meta3}.get(stem, EntryMetadata()) + def side_effect(path, **kwargs): + return { + Path("img1.jpg"): meta1, + Path("img2.jpg"): meta2, + Path("img3.jpg"): meta3, + }.get(Path(path), EntryMetadata()) mock_controller.sidecar.get_metadata.side_effect = side_effect diff --git a/faststack/tests/test_sidecar.py b/faststack/tests/test_sidecar.py index 2c97eae..bd0a28e 100644 --- a/faststack/tests/test_sidecar.py +++ b/faststack/tests/test_sidecar.py @@ -1,6 +1,7 @@ """Tests for the SidecarManager.""" import json +import os from pathlib import Path import pytest @@ -61,7 +62,7 @@ def test_sidecar_save(mock_sidecar_dir): # Modify data sm.set_last_index(10) - meta = sm.get_metadata("IMG_TEST") + meta = sm.get_metadata(Path("IMG_TEST.jpg")) # Modify a valid field meta.stack_id = 99 @@ -70,16 +71,18 @@ def test_sidecar_save(mock_sidecar_dir): # Verify file content saved_data = json.loads((d / "faststack.json").read_text()) + expected_key = sm.metadata_key_for_path(Path("IMG_TEST.jpg")) assert saved_data["last_index"] == 10 - assert saved_data["entries"]["IMG_TEST"]["stack_id"] == 99 + assert saved_data["entries"][expected_key]["stack_id"] == 99 def test_sidecar_get_metadata_creates_new(mock_sidecar_dir): """Tests that get_metadata creates a new entry if one doesn't exist.""" d = mock_sidecar_dir() sm = SidecarManager(d, None) - assert "NEW_IMG" not in sm.data.entries - meta = sm.get_metadata("NEW_IMG") + expected_key = sm.metadata_key_for_path(Path("NEW_IMG.jpg")) + assert expected_key not in sm.data.entries + meta = sm.get_metadata(Path("NEW_IMG.jpg")) # EntryMetadata may be a runtime class OR a typing alias, depending on refactors. if isinstance(EntryMetadata, type): @@ -89,14 +92,14 @@ def test_sidecar_get_metadata_creates_new(mock_sidecar_dir): assert meta.__class__.__name__ == "EntryMetadata" assert hasattr(meta, "stack_id") - assert "NEW_IMG" in sm.data.entries + assert expected_key in sm.data.entries def test_favorite_toggle_sets_json(mock_sidecar_dir): """Tests that toggling favorite writes true/false to JSON.""" d = mock_sidecar_dir() sm = SidecarManager(d, None) - meta = sm.get_metadata("IMG_FAV") + meta = sm.get_metadata(Path("IMG_FAV.jpg")) # Initially false assert meta.favorite is False @@ -105,13 +108,14 @@ def test_favorite_toggle_sets_json(mock_sidecar_dir): meta.favorite = True sm.save() saved = json.loads((d / "faststack.json").read_text()) - assert saved["entries"]["IMG_FAV"]["favorite"] is True + expected_key = sm.metadata_key_for_path(Path("IMG_FAV.jpg")) + assert saved["entries"][expected_key]["favorite"] is True # Toggle off meta.favorite = False sm.save() saved = json.loads((d / "faststack.json").read_text()) - assert saved["entries"]["IMG_FAV"]["favorite"] is False + assert saved["entries"][expected_key]["favorite"] is False def test_favorite_loads_from_sidecar(mock_sidecar_dir): @@ -120,12 +124,12 @@ def test_favorite_loads_from_sidecar(mock_sidecar_dir): "version": 2, "last_index": 0, "entries": { - "IMG_FAV": {"favorite": True}, + "IMG_FAV.jpg": {"favorite": True}, }, } d = mock_sidecar_dir(content) sm = SidecarManager(d, None) - meta = sm.get_metadata("IMG_FAV") + meta = sm.get_metadata(Path("IMG_FAV.jpg")) assert meta.favorite is True @@ -133,7 +137,7 @@ def test_favorite_toggle_roundtrip(mock_sidecar_dir): """Tests that toggling twice restores original JSON (round-trip).""" d = mock_sidecar_dir() sm = SidecarManager(d, None) - meta = sm.get_metadata("IMG_FAV") + meta = sm.get_metadata(Path("IMG_FAV.jpg")) # Capture original state assert meta.favorite is False @@ -145,5 +149,267 @@ def test_favorite_toggle_roundtrip(mock_sidecar_dir): # Reload and verify sm2 = SidecarManager(d, None) - meta2 = sm2.get_metadata("IMG_FAV") + meta2 = sm2.get_metadata(Path("IMG_FAV.jpg")) assert meta2.favorite is False + + +def test_legacy_stem_entry_migrates_to_path_key(mock_sidecar_dir): + """Legacy filename-keyed entries should migrate on first concrete path lookup.""" + # Use a legacy key ("IMG_0001.jpg") that differs from the stable key + # returned by metadata_key_for_path (which strips the extension), so that + # get_metadata must actually perform migration. + legacy_key = "IMG_0001.jpg" + content = { + "version": 2, + "entries": { + legacy_key: {"uploaded": True}, + }, + } + d = mock_sidecar_dir(content) + sm = SidecarManager(d, None) + + expected_key = sm.metadata_key_for_path(Path("IMG_0001.jpg")) + assert expected_key != legacy_key, "legacy key must differ from stable key" + + meta = sm.get_metadata(Path("IMG_0001.jpg"), create=False) + + assert meta is not None + assert meta.uploaded is True + # Entry now lives under the stable key + assert expected_key in sm.data.entries + # Legacy key was removed during migration + assert legacy_key not in sm.data.entries + + +def test_bulk_iteration_finds_legacy_keyed_entries(mock_sidecar_dir): + """Bulk get_metadata(path, create=False) must find and migrate legacy keys. + + Regression test: hot paths like flag filtering and batch selection iterate + over all images calling get_metadata(path, create=False). If they bypassed + migration (e.g. via direct entries dict access), legacy-keyed entries would + be silently missed. + """ + content = { + "version": 2, + "entries": { + # Legacy filename-keyed entry (has extension — differs from stable key) + "IMG_0001.jpg": {"favorite": True}, + # Already-stable entry (bare stem, matches metadata_key_for_path) + "IMG_0002": {"uploaded": True}, + # Another legacy filename-keyed entry + "IMG_0003.jpg": {"favorite": True, "uploaded": True}, + }, + } + d = mock_sidecar_dir(content) + sm = SidecarManager(d, None) + + # Simulate the bulk iteration pattern used by _apply_filter_to_cached_list, + # _get_bulk_metadata_map, _batch_add_favorites, _batch_add_uploaded + image_paths = [Path("IMG_0001.jpg"), Path("IMG_0002.jpg"), Path("IMG_0003.jpg")] + found = {} + for p in image_paths: + meta = sm.get_metadata(p, create=False) + if meta is not None: + found[p.stem] = meta + + # All three entries must be found — none silently dropped + assert "IMG_0001" in found, "legacy filename-keyed entry must be found" + assert "IMG_0002" in found, "already-stable entry must be found" + assert "IMG_0003" in found, "legacy filename-keyed entry must be found" + + assert found["IMG_0001"].favorite is True + assert found["IMG_0002"].uploaded is True + assert found["IMG_0003"].favorite is True + assert found["IMG_0003"].uploaded is True + + # Legacy keys should have been migrated to stable keys + stable_1 = sm.metadata_key_for_path(Path("IMG_0001.jpg")) + stable_3 = sm.metadata_key_for_path(Path("IMG_0003.jpg")) + assert stable_1 in sm.data.entries + assert stable_3 in sm.data.entries + assert "IMG_0001.jpg" not in sm.data.entries + assert "IMG_0003.jpg" not in sm.data.entries + + +def test_raw_only_entry_survives_transition_to_visible_jpg(mock_sidecar_dir): + """RAW-only metadata must remain visible after a matching JPG appears.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + + raw_meta = sm.get_metadata(Path("photo.CR2")) + raw_meta.favorite = True + raw_meta.uploaded = True + + jpg_meta = sm.get_metadata(Path("photo.jpg"), create=False) + + assert jpg_meta is raw_meta + assert jpg_meta.favorite is True + assert jpg_meta.uploaded is True + assert list(sm.data.entries) == ["photo"] + + +def test_regressed_filename_key_migrates_to_stable_stem_key(mock_sidecar_dir): + """Entries created by the filename-key regression should migrate back.""" + content = { + "version": 2, + "entries": { + "photo.CR2": {"favorite": True}, + }, + } + d = mock_sidecar_dir(content) + sm = SidecarManager(d, None) + + meta = sm.get_metadata(Path("photo.jpg"), create=False) + + assert meta is not None + assert meta.favorite is True + assert "photo" in sm.data.entries + assert "photo.CR2" not in sm.data.entries + assert "photo.jpg" not in sm.data.entries + + +def test_same_stem_in_different_subfolders_do_not_collide(mock_sidecar_dir): + """Relative parent path must namespace same-stem files in different folders.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + + first = sm.get_metadata(Path("a") / "photo.CR2") + first.favorite = True + + second = sm.get_metadata(Path("b") / "photo.jpg") + second.uploaded = True + + assert first is not second + assert sm.metadata_key_for_path(Path("a") / "photo.CR2") == "a/photo" + assert sm.metadata_key_for_path(Path("b") / "photo.jpg") == "b/photo" + assert sm.data.entries["a/photo"].favorite is True + assert sm.data.entries["a/photo"].uploaded is False + assert sm.data.entries["b/photo"].favorite is False + assert sm.data.entries["b/photo"].uploaded is True + + +def test_semantic_dot_key_is_not_treated_as_path(mock_sidecar_dir): + """Semantic keys like sample.v1 must not be truncated as filenames.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + legacy = EntryMetadata(todo=True) + sm.data.entries["sample.v1"] = legacy + + meta = sm.get_metadata("sample.v1", create=False) + + assert meta is legacy + assert "sample.v1" in sm.data.entries + assert "sample" not in sm.data.entries + + +def test_dotted_string_with_image_extension_stays_exact(mock_sidecar_dir): + """String key 'photo.CR2' must not be path-normalized to 'photo'.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + legacy = EntryMetadata(favorite=True) + sm.data.entries["photo.CR2"] = legacy + + meta = sm.get_metadata("photo.CR2", create=False) + + assert meta is legacy + assert "photo.CR2" in sm.data.entries + assert "photo" not in sm.data.entries + + +def test_dotted_string_with_jpg_extension_stays_exact(mock_sidecar_dir): + """String key 'IMG_0001.jpg' must not be path-normalized when used as string.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + legacy = EntryMetadata(uploaded=True) + sm.data.entries["IMG_0001.jpg"] = legacy + + meta = sm.get_metadata("IMG_0001.jpg", create=False) + + assert meta is legacy + assert "IMG_0001.jpg" in sm.data.entries + + +def test_string_with_path_separator_is_path_normalized(mock_sidecar_dir): + """String containing a path separator should be treated as a path.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + + meta = sm.get_metadata("subdir/photo.CR2") + expected_key = sm.metadata_key_for_path(Path("subdir/photo.CR2")) + + assert expected_key in sm.data.entries + assert expected_key == "subdir/photo" + + +def test_legacy_filename_key_migrates_via_path_lookup(mock_sidecar_dir): + """Legacy 'photo.CR2' entry migrates when looked up via Path('photo.jpg').""" + content = { + "version": 2, + "entries": { + "photo.CR2": {"favorite": True}, + }, + } + d = mock_sidecar_dir(content) + sm = SidecarManager(d, None) + + # Path-based lookup should find and migrate the legacy key + meta = sm.get_metadata(Path("photo.jpg"), create=False) + + assert meta is not None + assert meta.favorite is True + assert "photo" in sm.data.entries + assert "photo.CR2" not in sm.data.entries + + +def test_empty_string_create_false_returns_none(mock_sidecar_dir): + """Empty string refs must not create a sidecar entry.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + + assert sm.get_metadata("", create=False) is None + assert "" not in sm.data.entries + + +def test_empty_string_create_true_raises(mock_sidecar_dir): + """Empty string refs should fail fast when asked to create metadata.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + + with pytest.raises(ValueError, match="image_ref must not be empty"): + sm.get_metadata("", create=True) + + assert "" not in sm.data.entries + + +def test_empty_path_create_false_returns_none(mock_sidecar_dir): + """Path('') should be treated the same as an empty string.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + + assert sm.get_metadata(Path(""), create=False) is None + assert "" not in sm.data.entries + + +def test_empty_path_create_true_raises(mock_sidecar_dir): + """Path('') should fail fast when asked to create metadata.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + + with pytest.raises(ValueError, match="image_ref must not be empty"): + sm.get_metadata(Path(""), create=True) + + assert "" not in sm.data.entries + + +def test_metadata_key_for_path_normalizes_case_on_windows(mock_sidecar_dir): + """Windows path casing should not create duplicate stable keys.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + + key_upper = sm.metadata_key_for_path(Path("Mixed/Case/IMG_0001.JPG")) + key_lower = sm.metadata_key_for_path(Path("mixed/case/img_0001.jpg")) + + if os.name == "nt": + assert key_upper == key_lower + else: + assert key_upper != key_lower diff --git a/faststack/tests/thumbnail_view/test_folder_stats.py b/faststack/tests/thumbnail_view/test_folder_stats.py index 90866e9..fb36a10 100644 --- a/faststack/tests/thumbnail_view/test_folder_stats.py +++ b/faststack/tests/thumbnail_view/test_folder_stats.py @@ -344,6 +344,18 @@ def test_coverage_buckets_in_stats(self, temp_folder): # With 2 files and default 40 buckets, should have 2 buckets assert len(stats.coverage_buckets) == 2 + def test_coverage_buckets_support_path_keys(self, temp_folder): + """Path-aware sidecar keys should still contribute to sparkline coverage.""" + (temp_folder / "a.jpg").touch() + json_path = temp_folder / "faststack.json" + data = {"entries": {"a.jpg": {"uploaded": True}}} + json_path.write_text(json.dumps(data)) + + stats = read_folder_stats(temp_folder) + + assert stats is not None + assert stats.coverage_buckets == [(1.0, 0.0, 0.0)] + class TestCountImagesInFolder: """Tests for count_images_in_folder function.""" diff --git a/faststack/thumbnail_view/folder_stats.py b/faststack/thumbnail_view/folder_stats.py index 8711f06..de19a7e 100644 --- a/faststack/thumbnail_view/folder_stats.py +++ b/faststack/thumbnail_view/folder_stats.py @@ -247,7 +247,9 @@ def _compute_coverage_buckets( # Efficient stem extraction and metadata lookup stem, _ = os.path.splitext(filename) - meta = entries.get(stem) + meta = entries.get(filename) + if meta is None: + meta = entries.get(stem) if isinstance(meta, dict): if meta.get("uploaded", False): diff --git a/faststack/thumbnail_view/model.py b/faststack/thumbnail_view/model.py index fd08620..50c175a 100644 --- a/faststack/thumbnail_view/model.py +++ b/faststack/thumbnail_view/model.py @@ -128,9 +128,10 @@ def __init__( self, base_directory: Path, current_directory: Path, - get_metadata_callback: Optional[Callable[[str], dict]] = None, + get_metadata_callback: Optional[Callable[[Path | str], dict]] = None, get_batch_indices_callback: Optional[Callable[[], Set[int]]] = None, get_current_index_callback: Optional[Callable[[], int]] = None, + metadata_key_fn: Optional[Callable[[Path], str]] = None, thumbnail_size: int = 200, parent=None, ): @@ -141,6 +142,7 @@ def __init__( self._get_metadata = get_metadata_callback self._get_batch_indices = get_batch_indices_callback self._get_current_index = get_current_index_callback + self._metadata_key_fn = metadata_key_fn self._thumbnail_size = thumbnail_size self._entries: List[ThumbnailEntry] = [] self._folder_count: int = 0 # cached; updated on mutation @@ -266,9 +268,9 @@ def _get_loupe_index_for_entry(self, entry: ThumbnailEntry) -> Optional[int]: # We'll use the parent (AppController) to look this up parent = self.parent() if parent and hasattr(parent, "_path_to_index"): - # Must use the same key format as _rebuild_path_to_index (abspath, - # not realpath/resolve) so the lookup hits on Windows and Linux. - key = os.path.normcase(os.path.abspath(str(entry.path))) + # Must use the same key format as _rebuild_path_to_index + # (normalize_path_key) so the lookup hits on Windows and Linux. + key = normalize_path_key(entry.path) return parent._path_to_index.get(key) return None @@ -406,12 +408,10 @@ def refresh(self): filtered = [] for img in images: try: - meta = self._get_metadata(img.path.stem) + meta = self._get_metadata(img.path) if not isinstance(meta, dict): # Ensure it's a dict before .get() - log.debug( - "Metadata for %s is not a dict: %r", img.path.stem, meta - ) + log.debug("Metadata for %s is not a dict: %r", img.path, meta) continue if all(meta.get(flag, False) for flag in flags): @@ -454,10 +454,10 @@ def remove_rows_by_path(self, paths: List[Path]) -> None: return # 1. Map paths to rows (normalized for Windows case/separator consistency) - path_keys = {os.path.normcase(os.path.abspath(str(p))) for p in paths} + path_keys = {normalize_path_key(p) for p in paths} indices_to_remove = [] for i, entry in enumerate(self._entries): - if os.path.normcase(os.path.abspath(str(entry.path))) in path_keys: + if normalize_path_key(entry.path) in path_keys: indices_to_remove.append(i) if not indices_to_remove: @@ -549,16 +549,16 @@ def refresh_from_controller( filtered = [] for img in images: try: - if metadata_map: - meta = metadata_map.get(img.path.stem, {}) + if metadata_map and self._metadata_key_fn: + meta = metadata_map.get(self._metadata_key_fn(img.path), {}) elif self._get_metadata: - meta = self._get_metadata(img.path.stem) + meta = self._get_metadata(img.path) else: continue if not isinstance(meta, dict): log.debug( - "Metadata for %s is not a dict: %r", img.path.stem, meta + "Metadata for %s is not a dict: %r", img.path, meta ) continue @@ -611,8 +611,8 @@ def _add_images_to_entries( is_favorite = False is_todo = False - if metadata_map: - meta = metadata_map.get(img.path.stem, {}) + if metadata_map and self._metadata_key_fn: + meta = metadata_map.get(self._metadata_key_fn(img.path), {}) is_stacked = meta.get("stacked", False) is_uploaded = meta.get("uploaded", False) is_edited = meta.get("edited", False) @@ -621,7 +621,7 @@ def _add_images_to_entries( is_todo = meta.get("todo", False) elif self._get_metadata: try: - meta = self._get_metadata(img.path.stem) + meta = self._get_metadata(img.path) if isinstance(meta, dict): is_stacked = meta.get("stacked", False) is_uploaded = meta.get("uploaded", False) @@ -631,7 +631,7 @@ def _add_images_to_entries( is_todo = meta.get("todo", False) else: log.debug( - "Metadata for %s is not a dict: %r", img.path.stem, meta + "Metadata for %s is not a dict: %r", img.path, meta ) except Exception as e: log.debug("Error getting metadata for %s: %s", img.path, e)