From a8810e5da75801cdddbb5216045d3cfad0cadf5b Mon Sep 17 00:00:00 2001 From: Eric Pheterson Date: Wed, 27 May 2026 16:08:38 -0700 Subject: [PATCH 1/4] feat: per-library destination subdirectories (photos.library_destinations) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New optional config block: photos.library_destinations maps each iCloud library to a subdirectory of photos.destination. When unset (default), all libraries share the single photos.destination tree — preserving the historical mandarons/icloud-docker behaviour, no breaking change. Example: photos: destination: photos library_destinations: PrimarySync: personal SharedLibrary: shared Personal photos land in /personal/, Shared library photos in /shared/. Threaded through sync_photos.sync_photos, _sync_all_photos_first_for_hardlinks, and _sync_albums_by_configuration. New helper _library_destination resolves the per-library path. Obsolete-file cleanup walks each per-library subdir independently when library_destinations is set, falling back to the legacy single-destination walk otherwise. 10 new tests in tests/test_library_destinations.py covering: config helper defaults / dict / non-dict, _library_destination joins + creates dirs / handles None mapping / handles missing library / handles nested subdirs. --- src/config_parser.py | 30 +++++++++++ src/sync_photos.py | 44 +++++++++++++--- tests/test_library_destinations.py | 84 ++++++++++++++++++++++++++++++ 3 files changed, 151 insertions(+), 7 deletions(-) create mode 100644 tests/test_library_destinations.py diff --git a/src/config_parser.py b/src/config_parser.py index 4099b0766..d2c6bdd1f 100644 --- a/src/config_parser.py +++ b/src/config_parser.py @@ -599,6 +599,36 @@ def validate_file_sizes(file_sizes: list[str]) -> list[str]: return validated_sizes if validated_sizes else ["original"] +def get_photos_library_destinations(config: dict) -> dict[str, str]: + """Get per-library destination subdirectory mapping from photos config. + + Optional config block (under top-level ``photos``): + + .. code-block:: yaml + + photos: + destination: photos + library_destinations: + PrimarySync: personal + SharedLibrary: shared + + When set, photos from each library are written to + ``//...`` instead of + sharing one destination tree. When unset (the default), all libraries + share the single ``photos.destination`` path — preserving the historical + behaviour of mandarons/icloud-docker. + + Returns: + Dict mapping library name → subdirectory relative to ``photos.destination``. + Returns ``{}`` if not configured (backward-compatible default). + """ + config_path = ["photos", "library_destinations"] + mapping = get_config_value_or_none(config=config, config_path=config_path) + if not mapping or not isinstance(mapping, dict): + return {} + return {str(k): str(v) for k, v in mapping.items()} + + def get_photos_libraries_filter(config: dict, base_config_path: list[str]) -> list[str] | None: """Get libraries filter from photos config. diff --git a/src/sync_photos.py b/src/sync_photos.py index abedc0108..5a3bf0f0e 100644 --- a/src/sync_photos.py +++ b/src/sync_photos.py @@ -385,6 +385,7 @@ def sync_photos(config, photos): """ # Parse configuration using centralized config parser destination_path = config_parser.prepare_photos_destination(config=config) + library_destinations = config_parser.get_photos_library_destinations(config=config) filters = config_parser.get_photos_filters(config=config) files = set() download_all = config_parser.get_photos_all_albums(config=config) @@ -408,6 +409,7 @@ def sync_photos(config, photos): folder_format, hardlink_registry, config, + library_destinations=library_destinations, ) total_successful += sub_successful total_failed += sub_failed @@ -423,17 +425,41 @@ def sync_photos(config, photos): folder_format, hardlink_registry, config, + library_destinations=library_destinations, ) total_successful += sub_successful total_failed += sub_failed - # Clean up obsolete files if enabled + # Clean up obsolete files if enabled. When per-library destinations are + # configured we walk each library's subdir independently, otherwise the + # legacy single-destination walk preserves backward compatibility. if config_parser.get_photos_remove_obsolete(config=config): - remove_obsolete_files(destination_path, files) + if library_destinations: + for library in libraries: + lib_dest = _library_destination(destination_path, library, library_destinations) + remove_obsolete_files(lib_dest, files) + else: + remove_obsolete_files(destination_path, files) return total_successful, total_failed +def _library_destination(base_destination: str, library: str, library_destinations: dict) -> str: + """Resolve the on-disk destination for a given iCloud photo library. + + When ``library_destinations`` provides a mapping for ``library``, joins + the configured subdirectory under ``base_destination`` and ensures the + directory exists. Otherwise returns ``base_destination`` unchanged + (preserving mandarons' legacy single-destination behaviour). + """ + subdir = library_destinations.get(library) if library_destinations else None + if not subdir: + return base_destination + dest = os.path.join(base_destination, subdir) + os.makedirs(dest, exist_ok=True) + return dest + + def _sync_all_photos_first_for_hardlinks( photos, libraries, @@ -443,6 +469,7 @@ def _sync_all_photos_first_for_hardlinks( folder_format, hardlink_registry, config, + library_destinations: dict | None = None, ) -> tuple[int, int]: """Sync 'All Photos' album first to populate hardlink registry. @@ -462,9 +489,10 @@ def _sync_all_photos_first_for_hardlinks( for library in libraries: if library == "PrimarySync" and "All Photos" in photos.libraries[library].albums: LOGGER.info("Syncing 'All Photos' album first for hard link reference...") + lib_dest = _library_destination(destination_path, library, library_destinations or {}) result = sync_album_photos( album=photos.libraries[library].albums["All Photos"], - destination_path=os.path.join(destination_path, "All Photos"), + destination_path=os.path.join(lib_dest, "All Photos"), file_sizes=filters["file_sizes"], extensions=filters["extensions"], files=files, @@ -493,6 +521,7 @@ def _sync_albums_by_configuration( folder_format, hardlink_registry, config, + library_destinations: dict | None = None, ) -> tuple[int, int]: """Sync albums based on configuration settings. @@ -512,12 +541,13 @@ def _sync_albums_by_configuration( """ total_successful, total_failed = 0, 0 for library in libraries: + lib_dest = _library_destination(destination_path, library, library_destinations or {}) if download_all and library == "PrimarySync": sub_successful, sub_failed = _sync_all_albums_except_filtered( photos, library, filters, - destination_path, + lib_dest, files, folder_format, hardlink_registry, @@ -528,7 +558,7 @@ def _sync_albums_by_configuration( photos, library, filters, - destination_path, + lib_dest, files, folder_format, hardlink_registry, @@ -539,7 +569,7 @@ def _sync_albums_by_configuration( photos, library, filters, - destination_path, + lib_dest, files, folder_format, hardlink_registry, @@ -549,7 +579,7 @@ def _sync_albums_by_configuration( sub_successful, sub_failed = _sync_all_photos_in_library( photos, library, - destination_path, + lib_dest, filters, files, folder_format, diff --git a/tests/test_library_destinations.py b/tests/test_library_destinations.py new file mode 100644 index 000000000..390d30f79 --- /dev/null +++ b/tests/test_library_destinations.py @@ -0,0 +1,84 @@ +"""Tests for per-library destination subdirectories. + +Added 2026-05-27 as part of feat/per-library-destinations-and-live-photos. +Covers ``config_parser.get_photos_library_destinations`` and the +``_library_destination`` helper threaded through sync_photos. +""" + +import os +import tempfile +import unittest + +from src import config_parser +from src.sync_photos import _library_destination + + +class TestGetPhotosLibraryDestinations(unittest.TestCase): + """get_photos_library_destinations returns dict or {} based on config.""" + + def test_returns_empty_dict_when_unset(self): + config = {"photos": {"destination": "photos"}} + assert config_parser.get_photos_library_destinations(config) == {} + + def test_returns_empty_dict_when_photos_section_missing(self): + config = {} + assert config_parser.get_photos_library_destinations(config) == {} + + def test_returns_mapping_when_configured(self): + config = { + "photos": { + "destination": "photos", + "library_destinations": { + "PrimarySync": "personal", + "SharedLibrary": "shared", + }, + } + } + result = config_parser.get_photos_library_destinations(config) + assert result == {"PrimarySync": "personal", "SharedLibrary": "shared"} + + def test_returns_empty_dict_when_value_is_not_a_dict(self): + config = {"photos": {"library_destinations": ["foo", "bar"]}} + assert config_parser.get_photos_library_destinations(config) == {} + + def test_coerces_non_string_keys_and_values_to_str(self): + config = { + "photos": {"library_destinations": {123: 456, "PrimarySync": "personal"}} + } + result = config_parser.get_photos_library_destinations(config) + assert result == {"123": "456", "PrimarySync": "personal"} + + +class TestLibraryDestinationHelper(unittest.TestCase): + """_library_destination resolves the right on-disk path per library.""" + + def test_returns_base_when_no_mapping(self): + with tempfile.TemporaryDirectory() as base: + result = _library_destination(base, "PrimarySync", {}) + assert result == base + + def test_returns_base_when_library_not_in_mapping(self): + with tempfile.TemporaryDirectory() as base: + mapping = {"PrimarySync": "personal"} + result = _library_destination(base, "SharedLibrary", mapping) + # Library not in mapping → fall through to base destination + assert result == base + + def test_joins_subdir_and_creates_directory(self): + with tempfile.TemporaryDirectory() as base: + mapping = {"PrimarySync": "personal"} + result = _library_destination(base, "PrimarySync", mapping) + assert result == os.path.join(base, "personal") + assert os.path.isdir(result) + + def test_creates_nested_subdirectories(self): + with tempfile.TemporaryDirectory() as base: + mapping = {"PrimarySync": "by-source/personal"} + result = _library_destination(base, "PrimarySync", mapping) + assert result == os.path.join(base, "by-source", "personal") + assert os.path.isdir(result) + + def test_none_mapping_is_safe(self): + with tempfile.TemporaryDirectory() as base: + result = _library_destination(base, "PrimarySync", None) + assert result == base From eea0d739265f458b6545529673470fb6b490d499 Mon Sep 17 00:00:00 2001 From: Eric Pheterson Date: Thu, 28 May 2026 16:15:51 -0700 Subject: [PATCH 2/4] feat(photos): SharedLibrary alias matches Apple's GUID-named shared zones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to a8810e5da. Apple's modern iCloud Shared Photo Library is exposed by icloudpy under a GUID-based zone name like 'SharedSync-3C977B4A-C15A-46E4-9854-585B9342C409', not the literal string 'SharedLibrary' that early mandarons docs (and the v1 of this feature's README) used as the example. Without this fix, a user whose config follows the documented example ('SharedLibrary: Shared') would silently fall through to the base destination — shared-library photos would land alongside primary photos in /icloud/photos/ instead of in /icloud/photos/Shared/. Caught during a real boredazfcuk migration: dry-run output reported 'Photos libraries available: SharedSync-3C977B4A-..., PrimarySync' while the config expected the literal 'SharedLibrary'. _library_destination now has three rules, in priority order: 1. Exact match (unchanged). 2. If the zone name starts with 'SharedSync-' and config has a 'SharedLibrary' key, use that (new alias rule). 3. Fall through to base destination (unchanged). Three new tests in tests/test_library_destinations.py cover the alias behaviour: GUID zone matched via SharedLibrary key, alias does NOT catch non-SharedSync libraries, and exact-GUID config wins over the alias (predictability for users who pinned the GUID before this feature shipped). The base PR (a8810e5da) and this follow-up will land as a single PR upstream; preserved as a separate commit here so the alias delta is reviewable in isolation against the original feature. --- src/sync_photos.py | 18 +++++++++++++++- tests/test_library_destinations.py | 33 +++++++++++++++++++++++++++--- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/src/sync_photos.py b/src/sync_photos.py index 5a3bf0f0e..b7a4d9c08 100644 --- a/src/sync_photos.py +++ b/src/sync_photos.py @@ -451,8 +451,24 @@ def _library_destination(base_destination: str, library: str, library_destinatio the configured subdirectory under ``base_destination`` and ensures the directory exists. Otherwise returns ``base_destination`` unchanged (preserving mandarons' legacy single-destination behaviour). + + Library-name matching has three rules, in priority order: + + 1. **Exact match.** ``library_destinations[library]`` if present. + 2. **Role alias for `SharedLibrary`.** Apple's modern iCloud Shared + Photo Library is exposed by icloudpy under a GUID-based zone name + like ``SharedSync-3C977B4A-C15A-46E4-9854-585B9342C409``. A config + key of ``SharedLibrary`` matches any zone whose name starts with + ``SharedSync-`` so users don't need to discover and hardcode the + per-account GUID. (Configs that already use the literal current + Apple zone name still work via rule 1.) + 3. **Fallthrough.** Returns ``base_destination`` unchanged. """ - subdir = library_destinations.get(library) if library_destinations else None + if not library_destinations: + return base_destination + subdir = library_destinations.get(library) + if not subdir and library.startswith("SharedSync-"): + subdir = library_destinations.get("SharedLibrary") if not subdir: return base_destination dest = os.path.join(base_destination, subdir) diff --git a/tests/test_library_destinations.py b/tests/test_library_destinations.py index 390d30f79..0464d7970 100644 --- a/tests/test_library_destinations.py +++ b/tests/test_library_destinations.py @@ -42,9 +42,7 @@ def test_returns_empty_dict_when_value_is_not_a_dict(self): assert config_parser.get_photos_library_destinations(config) == {} def test_coerces_non_string_keys_and_values_to_str(self): - config = { - "photos": {"library_destinations": {123: 456, "PrimarySync": "personal"}} - } + config = {"photos": {"library_destinations": {123: 456, "PrimarySync": "personal"}}} result = config_parser.get_photos_library_destinations(config) assert result == {"123": "456", "PrimarySync": "personal"} @@ -82,3 +80,32 @@ def test_none_mapping_is_safe(self): with tempfile.TemporaryDirectory() as base: result = _library_destination(base, "PrimarySync", None) assert result == base + + def test_shared_library_alias_matches_guid_named_zone(self): + """`SharedLibrary` in the config matches Apple's GUID-named shared zones + (e.g. ``SharedSync-3C977B4A-...``) — users don't have to discover and + hardcode their per-account GUID.""" + with tempfile.TemporaryDirectory() as base: + mapping = {"PrimarySync": "Eric", "SharedLibrary": "Shared"} + result = _library_destination(base, "SharedSync-3C977B4A-C15A-46E4-9854-585B9342C409", mapping) + assert result == os.path.join(base, "Shared") + assert os.path.isdir(result) + + def test_shared_library_alias_only_fires_for_sharedsync_prefix(self): + """The alias rule does NOT silently catch unrelated library names — + a non-SharedSync library that isn't explicitly mapped still falls + through to base.""" + with tempfile.TemporaryDirectory() as base: + mapping = {"PrimarySync": "Eric", "SharedLibrary": "Shared"} + result = _library_destination(base, "OtherLibrary", mapping) + assert result == base + + def test_exact_match_wins_over_shared_library_alias(self): + """If the exact GUID is mapped explicitly, that wins over the + ``SharedLibrary`` alias — predictability for users who pinned the + GUID before this feature shipped.""" + with tempfile.TemporaryDirectory() as base: + guid_name = "SharedSync-DEADBEEF-1234-5678-9ABC-DEF012345678" + mapping = {guid_name: "Exact", "SharedLibrary": "Aliased"} + result = _library_destination(base, guid_name, mapping) + assert result == os.path.join(base, "Exact") From 84975eb0ae7a2098180b31e0105ba66db0fae2d9 Mon Sep 17 00:00:00 2001 From: Eric Pheterson Date: Sat, 30 May 2026 00:58:02 -0700 Subject: [PATCH 3/4] test: ruff-clean auto-fix (trailing commas etc.) Upstream CI runs ruff as part of the test step; the prior commit had auto-fixable lint issues that ruff caught. Pure formatting, no semantic change. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/test_library_destinations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_library_destinations.py b/tests/test_library_destinations.py index 0464d7970..55205a3bf 100644 --- a/tests/test_library_destinations.py +++ b/tests/test_library_destinations.py @@ -32,7 +32,7 @@ def test_returns_mapping_when_configured(self): "PrimarySync": "personal", "SharedLibrary": "shared", }, - } + }, } result = config_parser.get_photos_library_destinations(config) assert result == {"PrimarySync": "personal", "SharedLibrary": "shared"} From 7160f6e6619f098de8df2f6fc9850576b9b17bd7 Mon Sep 17 00:00:00 2001 From: Eric Pheterson Date: Sat, 30 May 2026 01:26:39 -0700 Subject: [PATCH 4/4] =?UTF-8?q?test:=20cover=20the=20per-library=20remove?= =?UTF-8?q?=5Fobsolete=20branch=20(lift=20coverage=2099.86%=20=E2=86=92=20?= =?UTF-8?q?100%)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upstream CI gates at 100% coverage; the new `if library_destinations: for library in libraries: remove_obsolete_files(...)` branch in sync_photos.sync_photos was the last 3 uncovered lines on this branch (because the existing remove_obsolete tests all hit the legacy single-destination path). Added TestSyncPhotosRemoveObsoletePerLibrary verifying remove_obsolete_files is invoked once per library_destinations subdir (not once on the parent — that would walk siblings' photo trees). Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/test_library_destinations.py | 60 +++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 2 deletions(-) diff --git a/tests/test_library_destinations.py b/tests/test_library_destinations.py index 55205a3bf..a73c0920f 100644 --- a/tests/test_library_destinations.py +++ b/tests/test_library_destinations.py @@ -42,7 +42,9 @@ def test_returns_empty_dict_when_value_is_not_a_dict(self): assert config_parser.get_photos_library_destinations(config) == {} def test_coerces_non_string_keys_and_values_to_str(self): - config = {"photos": {"library_destinations": {123: 456, "PrimarySync": "personal"}}} + config = { + "photos": {"library_destinations": {123: 456, "PrimarySync": "personal"}}, + } result = config_parser.get_photos_library_destinations(config) assert result == {"123": "456", "PrimarySync": "personal"} @@ -87,7 +89,9 @@ def test_shared_library_alias_matches_guid_named_zone(self): hardcode their per-account GUID.""" with tempfile.TemporaryDirectory() as base: mapping = {"PrimarySync": "Eric", "SharedLibrary": "Shared"} - result = _library_destination(base, "SharedSync-3C977B4A-C15A-46E4-9854-585B9342C409", mapping) + result = _library_destination( + base, "SharedSync-3C977B4A-C15A-46E4-9854-585B9342C409", mapping, + ) assert result == os.path.join(base, "Shared") assert os.path.isdir(result) @@ -109,3 +113,55 @@ def test_exact_match_wins_over_shared_library_alias(self): mapping = {guid_name: "Exact", "SharedLibrary": "Aliased"} result = _library_destination(base, guid_name, mapping) assert result == os.path.join(base, "Exact") + + +class TestSyncPhotosRemoveObsoletePerLibrary(unittest.TestCase): + """When ``library_destinations`` is configured AND + ``photos.remove_obsolete`` is true, ``sync_photos`` must call the + obsolete-cleanup once per library subdir (not once on the parent + destination — that would walk siblings' photo trees and delete them).""" + + def test_per_library_remove_obsolete_invokes_once_per_subdir(self): + from unittest.mock import MagicMock, patch + + from src import sync_photos as sp + + with tempfile.TemporaryDirectory() as base: + config = { + "photos": { + "destination": base, + "remove_obsolete": True, + "library_destinations": { + "PrimarySync": "personal", + "SharedLibrary": "shared", + }, + "filters": { + "libraries": ["PrimarySync", "SharedLibrary"], + "file_sizes": ["original"], + }, + }, + } + fake_photos = MagicMock() + fake_photos.libraries = { + "PrimarySync": MagicMock(), + "SharedLibrary": MagicMock(), + } + + with patch.object( + sp.config_parser, + "prepare_photos_destination", + return_value=base, + ), patch.object( + sp, + "_sync_albums_by_configuration", + return_value=(0, 0), + ), patch.object( + sp, "remove_obsolete_files", + ) as fake_remove: + sp.sync_photos(config=config, photos=fake_photos) + + called_paths = [c.args[0] for c in fake_remove.call_args_list] + assert called_paths == [ + os.path.join(base, "personal"), + os.path.join(base, "shared"), + ]