Fix issue #926 keyerror adjusted alternative (#1235)

Author: AndreyNikiforov

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- fix: KeyError when downloading photos with --size adjusted --size alternative options [#926](https://github.com/icloud-photos-downloader/icloud_photos_downloader/issues/926)
+
 ## 1.32.0 (2025-08-29)
 
 - feat: support multiple user configurations in single command [#1067](https://github.com/icloud-photos-downloader/icloud_photos_downloader/issues/1067) [#923](https://github.com/icloud-photos-downloader/icloud_photos_downloader/issues/923)

src/foundation/py.typed

@@ -70,17 +70,19 @@ def autodelete_photos(
 
         paths: Set[str] = set({})
         _size: VersionSize
-        for _size, _version in disambiguate_filenames(media.versions, _sizes).items():
+        for _size, _version in disambiguate_filenames(media.versions, _sizes, media).items():
             if _size in [AssetVersionSize.ALTERNATIVE, AssetVersionSize.ADJUSTED]:
-                paths.add(os.path.normpath(local_download_path(_version.filename, download_dir)))
+                version_filename = media.calculate_version_filename(_version, _size)
+                paths.add(os.path.normpath(local_download_path(version_filename, download_dir)))
                 paths.add(
-                    os.path.normpath(local_download_path(_version.filename, download_dir)) + ".xmp"
+                    os.path.normpath(local_download_path(version_filename, download_dir)) + ".xmp"
                 )
         for _size, _version in media.versions.items():
             if _size not in [AssetVersionSize.ALTERNATIVE, AssetVersionSize.ADJUSTED]:
-                paths.add(os.path.normpath(local_download_path(_version.filename, download_dir)))
+                version_filename = media.calculate_version_filename(_version, _size)
+                paths.add(os.path.normpath(local_download_path(version_filename, download_dir)))
                 paths.add(
-                    os.path.normpath(local_download_path(_version.filename, download_dir)) + ".xmp"
+                    os.path.normpath(local_download_path(version_filename, download_dir)) + ".xmp"
                 )
         for path in paths:
             if os.path.exists(path):

src/icloudpd/autodelete.py

@@ -55,6 +55,7 @@
 from icloudpd.status import Status, StatusExchange
 from icloudpd.string_helpers import parse_timestamp_or_timedelta, truncate_middle
 from icloudpd.xmp_sidecar import generate_xmp_file
+from pyicloud_ipd.asset_version import add_suffix_to_filename
 from pyicloud_ipd.base import PyiCloudService
 from pyicloud_ipd.exceptions import (
     PyiCloudAPIResponseException,
@@ -68,7 +69,6 @@
 from pyicloud_ipd.live_photo_mov_filename_policy import LivePhotoMovFilenamePolicy
 from pyicloud_ipd.services.photos import PhotoAlbum, PhotoAsset, PhotoLibrary
 from pyicloud_ipd.utils import (
-    add_suffix_to_filename,
     disambiguate_filenames,
     get_password_from_keyring,
     size_to_suffix,
@@ -554,7 +554,7 @@ def download_builder(
             date_path = folder_structure.format(created_date)
 
     try:
-        versions = disambiguate_filenames(photo.versions, primary_sizes)
+        versions = disambiguate_filenames(photo.versions, primary_sizes, photo)
     except KeyError as ex:
         print(f"KeyError: {ex} attribute was not found in the photo fields.")
         with open(file="icloudpd-photo-error.json", mode="w", encoding="utf8") as outfile:
@@ -591,7 +591,7 @@ def download_builder(
             download_size = AssetVersionSize.ORIGINAL
 
         version = versions[download_size]
-        filename = version.filename
+        filename = photo.calculate_version_filename(version, download_size)
 
         download_path = local_download_path(filename, download_dir)
 
@@ -657,7 +657,7 @@ def download_builder(
         lp_size = live_photo_size
         if lp_size in photo.versions:
             version = photo.versions[lp_size]
-            lp_filename = version.filename
+            lp_filename = photo.calculate_version_filename(version, lp_size)
             if live_photo_size != LivePhotoVersionSize.ORIGINAL:
                 # Add size to filename if not original
                 lp_filename = add_suffix_to_filename(

src/icloudpd/base.py

@@ -141,7 +141,9 @@ def download_media(
                 )
             else:
                 logger.error(
-                    "Could not find URL to download %s for size %s", version.filename, size.value
+                    "Could not find URL to download %s for size %s",
+                    photo.calculate_version_filename(version, size),
+                    size.value,
                 )
                 break
 

src/icloudpd/download.py

@@ -1,16 +1,77 @@
-from typing import Union
+import os
+from typing import Callable, Dict
+
+from pyicloud_ipd.version_size import VersionSize
+
+
+def add_suffix_to_filename(suffix: str, filename: str) -> str:
+    """Add suffix to filename before extension."""
+    _n, _e = os.path.splitext(filename)
+    return _n + suffix + _e
 
 
 class AssetVersion:
-    def __init__(self, filename: str, size: int, url: str, type: str, checksum: str) -> None:
-        self.filename = filename
+    def __init__(self, size: int, url: str, type: str, checksum: str) -> None:
         self.size = size
         self.url = url
         self.type = type
         self.checksum = checksum
+        self._filename_override: str | None = None
+
+    @property
+    def filename_override(self) -> str | None:
+        """Override filename used for disambiguation purposes."""
+        return self._filename_override
+    
+    @filename_override.setter
+    def filename_override(self, value: str | None) -> None:
+        """Set override filename for disambiguation purposes."""
+        self._filename_override = value
 
     def __eq__(self, other: object) -> bool: 
         if not isinstance(other, AssetVersion):
             # don't attempt to compare against unrelated types
             return NotImplemented
-        return self.filename == other.filename and self.size == other.size and self.url == other.url and self.type == other.type
+        return self.size == other.size and self.url == other.url and self.type == other.type
+
+
+def calculate_asset_version_filename(
+    base_filename: str,
+    asset_type: str,
+    version_size: VersionSize,
+    lp_filename_generator: Callable[[str], str],
+    item_type_extensions: Dict[str, str],
+    version_filename_suffix_lookup: Dict[VersionSize, str],
+    is_image_item_type: bool = True
+) -> str:
+    """
+    Calculate filename for an AssetVersion based on the base filename and version properties.
+    
+    Args:
+        base_filename: The base filename from PhotoAsset
+        asset_type: The type field from the version
+        version_size: The size key for this version
+        lp_filename_generator: Function to generate live photo filenames
+        item_type_extensions: Mapping of types to extensions
+        version_filename_suffix_lookup: Mapping of sizes to filename suffixes
+        is_image_item_type: Whether the parent asset is an image (for live photo detection)
+        
+    Returns:
+        The calculated filename for this asset version
+    """
+    filename = base_filename
+    
+    # Change live photo movie file extension to .MOV
+    if is_image_item_type and asset_type == "com.apple.quicktime-movie":
+        filename = lp_filename_generator(base_filename)  # without size
+    else:
+        # for non live photo movie, try to change file type to match asset type
+        _f, _e = os.path.splitext(filename)
+        filename = _f + "." + item_type_extensions.get(asset_type, _e[1:])
+
+    # add size suffix
+    if version_size in version_filename_suffix_lookup:
+        _size_suffix = version_filename_suffix_lookup[version_size]
+        filename = add_suffix_to_filename(f"-{_size_suffix}", filename)
+
+    return filename

src/icloudpd/py.typed

@@ -13,7 +13,7 @@
 from requests import Response
 from foundation import wrap_param_in_exception, bytes_decode
 from foundation.core import compose, identity
-from pyicloud_ipd.asset_version import AssetVersion
+from pyicloud_ipd.asset_version import AssetVersion, calculate_asset_version_filename, add_suffix_to_filename
 from pyicloud_ipd.exceptions import PyiCloudServiceNotActivatedException
 from pyicloud_ipd.exceptions import PyiCloudAPIResponseException
 
@@ -25,7 +25,6 @@
 from pyicloud_ipd.item_type import AssetItemType
 from pyicloud_ipd.raw_policy import RawTreatmentPolicy
 from pyicloud_ipd.session import PyiCloudSession
-from pyicloud_ipd.utils import add_suffix_to_filename
 from pyicloud_ipd.version_size import AssetVersionSize, LivePhotoVersionSize, VersionSize
 
 from tzlocal import get_localzone
@@ -720,6 +719,21 @@ def item_type_extension(self) -> str:
             return self.ITEM_TYPE_EXTENSIONS[item_type]
         return 'unknown'
 
+    def calculate_version_filename(self, version: AssetVersion, version_size: VersionSize) -> str:
+        """Calculate filename for a specific asset version."""
+        if version.filename_override is not None:
+            return version.filename_override
+            
+        return calculate_asset_version_filename(
+            self.filename,
+            version.type,
+            version_size,
+            self._service.lp_filename_generator,
+            self.ITEM_TYPE_EXTENSIONS,
+            self.VERSION_FILENAME_SUFFIX_LOOKUP,
+            (self.item_type or AssetItemType.IMAGE) == AssetItemType.IMAGE
+        )
+
     @property
     def versions(self) -> Dict[VersionSize, AssetVersion]:
         if not self._versions:
@@ -738,52 +752,21 @@ def versions(self) -> Dict[VersionSize, AssetVersion]:
                 if not f and '%sRes' % prefix in self._master_record['fields']:
                     f = self._master_record['fields']
                 if f:
-                    version: Dict[str, Any] = {'filename': self.filename}
-
-                    # width_entry = f.get('%sWidth' % prefix)
-                    # if width_entry:
-                    #     version['width'] = width_entry['value']
-                    # else:
-                    #     version['width'] = None
-
-                    # height_entry = f.get('%sHeight' % prefix)
-                    # if height_entry:
-                    #     version['height'] = height_entry['value']
-                    # else:
-                    #     version['height'] = None
-
                     size_entry = f.get('%sRes' % prefix)
                     if size_entry:
-                        version['size'] = size_entry['value']['size']
-                        version['url'] = size_entry['value']['downloadURL']
-                        version['checksum'] = size_entry['value']['fileChecksum']
+                        size = size_entry['value']['size']
+                        url = size_entry['value']['downloadURL']
+                        checksum = size_entry['value']['fileChecksum']
                     else:
                         raise ValueError(f"Expected {prefix}Res, but missing it")
-                        # version['size'] = None
-                        # version['url'] = None
 
                     type_entry = f.get('%sFileType' % prefix)
                     if type_entry:
-                        version['type'] = type_entry['value']
+                        asset_type = type_entry['value']
                     else:
                         raise ValueError(f"Expected {prefix}FileType, but missing it")
-                        # version['type'] = None
-
-                    # Change live photo movie file extension to .MOV
-                    if ((self.item_type or AssetItemType.IMAGE) == AssetItemType.IMAGE and
-                        version['type'] == "com.apple.quicktime-movie"):
-                        version['filename'] = self._service.lp_filename_generator(self.filename) # without size
-                    else:
-                        # for non live photo movie, try to change file type to match asset type
-                        _f, _e = os.path.splitext(version["filename"])
-                        version["filename"] = _f + "." + self.ITEM_TYPE_EXTENSIONS.get(version["type"], _e[1:])
-
-                    # add size suffix
-                    if key in self.VERSION_FILENAME_SUFFIX_LOOKUP:
-                        _size_suffix = self.VERSION_FILENAME_SUFFIX_LOOKUP[key]
-                        version["filename"] = add_suffix_to_filename(f"-{_size_suffix}", version["filename"])
 
-                    _versions[key] = AssetVersion(version["filename"], version['size'], version['url'], version['type'], version['checksum'])
+                    _versions[key] = AssetVersion(size, url, asset_type, checksum)
 
             # swap original & alternative according to swap_raw_policy
             if AssetVersionSize.ALTERNATIVE in _versions and (("raw" in _versions[AssetVersionSize.ALTERNATIVE].type and self._service.raw_policy == RawTreatmentPolicy.AS_ORIGINAL) or ("raw" in _versions[AssetVersionSize.ORIGINAL].type and self._service.raw_policy == RawTreatmentPolicy.AS_ALTERNATIVE)):

src/pyicloud_ipd/asset_version.py

@@ -1,15 +1,18 @@
 import copy
 import os
-from typing import Dict, Optional, Sequence
+from typing import TYPE_CHECKING, Dict, Optional, Sequence
 import typing
 import keyring
 from requests import Response
 
-from pyicloud_ipd.asset_version import AssetVersion
+from pyicloud_ipd.asset_version import AssetVersion, add_suffix_to_filename
 from pyicloud_ipd.version_size import AssetVersionSize, VersionSize
 
 from .exceptions import PyiCloudNoStoredPasswordAvailableException, PyiCloudServiceUnavailableException
 
+if TYPE_CHECKING:
+    from pyicloud_ipd.services.photos import PhotoAsset
+
 KEYRING_SYSTEM = 'pyicloud://icloud-password'
 
 
@@ -91,11 +94,8 @@ def underscore_to_camelcase(word:str , initial_capital: bool=False) -> str:
 def size_to_suffix(size: VersionSize) -> str:
     return f"-{size}".lower()
 
-def add_suffix_to_filename(suffix: str, filename: str) -> str:
-    _n, _e = os.path.splitext(filename)
-    return _n + suffix + _e
-
-def disambiguate_filenames(_versions: Dict[VersionSize, AssetVersion], _sizes:Sequence[AssetVersionSize]) -> Dict[AssetVersionSize, AssetVersion]:
+def disambiguate_filenames(_versions: Dict[VersionSize, AssetVersion], _sizes:Sequence[AssetVersionSize], photo_asset: 'PhotoAsset') -> Dict[AssetVersionSize, AssetVersion]:
+    
     _results: Dict[AssetVersionSize, AssetVersion] = {}
     # add those that were requested
     for _size in _sizes:
@@ -110,19 +110,33 @@ def disambiguate_filenames(_versions: Dict[VersionSize, AssetVersion], _sizes:Se
                 # clone
                 _results[AssetVersionSize.ADJUSTED] = copy.copy(_versions[AssetVersionSize.ORIGINAL])
         else:
-            if AssetVersionSize.ADJUSTED in _results and _results[AssetVersionSize.ORIGINAL].filename == _results[AssetVersionSize.ADJUSTED].filename:
-                _results[AssetVersionSize.ADJUSTED].filename = add_suffix_to_filename("-adjusted", _results[AssetVersionSize.ADJUSTED].filename)
+            if AssetVersionSize.ADJUSTED in _results:
+                original_filename = photo_asset.calculate_version_filename(_results[AssetVersionSize.ORIGINAL], AssetVersionSize.ORIGINAL)
+                adjusted_filename = photo_asset.calculate_version_filename(_results[AssetVersionSize.ADJUSTED], AssetVersionSize.ADJUSTED)
+                if original_filename == adjusted_filename:
+                    # Set filename override for adjusted version
+                    _results[AssetVersionSize.ADJUSTED].filename_override = add_suffix_to_filename("-adjusted", adjusted_filename)
 
     # alternative
     if AssetVersionSize.ALTERNATIVE in _sizes:
-        if AssetVersionSize.ORIGINAL not in _sizes and AssetVersionSize.ADJUSTED not in _results:
-            if AssetVersionSize.ALTERNATIVE not in _results:
-                # clone
+        if AssetVersionSize.ALTERNATIVE not in _results:
+            # Only clone from original when alternative is missing AND original is not requested
+            if AssetVersionSize.ORIGINAL not in _sizes:
                 _results[AssetVersionSize.ALTERNATIVE] = copy.copy(_versions[AssetVersionSize.ORIGINAL])
         else:
-            if AssetVersionSize.ALTERNATIVE in _results:
-                if AssetVersionSize.ADJUSTED in _results and _results[AssetVersionSize.ADJUSTED].filename == _results[AssetVersionSize.ALTERNATIVE].filename or AssetVersionSize.ORIGINAL in _results and _results[AssetVersionSize.ORIGINAL].filename == _results[AssetVersionSize.ALTERNATIVE].filename:
-                    _results[AssetVersionSize.ALTERNATIVE].filename = add_suffix_to_filename("-alternative", _results[AssetVersionSize.ALTERNATIVE].filename)
+            # Check for filename conflicts and add disambiguating suffix if needed
+            alternative_filename = photo_asset.calculate_version_filename(_results[AssetVersionSize.ALTERNATIVE], AssetVersionSize.ALTERNATIVE)
+            alt_adjusted_filename: Optional[str] = None
+            alt_original_filename: Optional[str] = None
+            
+            if AssetVersionSize.ADJUSTED in _results:
+                alt_adjusted_filename = photo_asset.calculate_version_filename(_results[AssetVersionSize.ADJUSTED], AssetVersionSize.ADJUSTED)
+            if AssetVersionSize.ORIGINAL in _results:
+                alt_original_filename = photo_asset.calculate_version_filename(_results[AssetVersionSize.ORIGINAL], AssetVersionSize.ORIGINAL)
+                
+            if (alt_adjusted_filename and alternative_filename == alt_adjusted_filename) or (alt_original_filename and alternative_filename == alt_original_filename):
+                # Set filename override for alternative version
+                _results[AssetVersionSize.ALTERNATIVE].filename_override = add_suffix_to_filename("-alternative", alternative_filename)
 
     for _size in _sizes:
         if _size not in [AssetVersionSize.ORIGINAL, AssetVersionSize.ADJUSTED, AssetVersionSize.ALTERNATIVE]:
@@ -134,7 +148,6 @@ def disambiguate_filenames(_versions: Dict[VersionSize, AssetVersion], _sizes:Se
             #     _n, _e = os.path.splitext(_results[_size]["filename"])
             #     _results[_size]["filename"] = f"{_n}-{_size}{_e}"
 
-
     return _results
 
 def throw_on_503(response: Response) -> Response: