Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions icloudpy/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
FindMyiPhoneServiceManager,
PhotosService,
RemindersService,
SharedPhotosService,
)
from icloudpy.utils import get_password_from_keyring

Expand Down Expand Up @@ -642,6 +643,14 @@ def photos(self):
self._photos = PhotosService(service_root, self.session, self.params)
return self._photos

@property
def shared_photos(self):
"""Gets the Shared 'Photo' service."""
if not self._photos:
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition if not self._photos: should check if not self._shared_photos: instead. This bug causes the method to incorrectly check the wrong instance variable, which would prevent proper lazy initialization of the shared photos service and could cause the regular photos service to be incorrectly overwritten.

Suggested change
if not self._photos:
if not self._shared_photos:

Copilot uses AI. Check for mistakes.
service_root = self._get_webservice_url("ckdatabasews")
self._shared_photos = SharedPhotosService(service_root, self.session, self.params)
return self._shared_photos
Comment on lines +646 to +652
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The instance variable self._shared_photos is never initialized. It should be initialized to None in the __init__ method (around line 273 where self._photos = None is set) to follow the same pattern as other lazy-loaded services like _photos and _drive.

Copilot uses AI. Check for mistakes.

@property
def calendar(self):
"""Gets the 'Calendar' service."""
Expand Down
2 changes: 1 addition & 1 deletion icloudpy/services/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from icloudpy.services.findmyiphone import (
FindMyiPhoneServiceManager,
)
from icloudpy.services.photos import PhotosService
from icloudpy.services.photos import PhotosService, SharedPhotosService
from icloudpy.services.reminders import (
RemindersService,
)
65 changes: 65 additions & 0 deletions icloudpy/services/photos.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,71 @@ def libraries(self):

return self._libraries

class SharedPhotosService(PhotoLibrary):
"""The 'Photos' iCloud service.

This also acts as the shared library if I am not the owner of the library.
Comment on lines +278 to +280
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring says "The 'Photos' iCloud service" which is identical to PhotosService. This should be more specific, such as "The 'Shared Photos' iCloud service" to clearly differentiate it from the regular PhotosService and accurately describe its purpose.

Suggested change
"""The 'Photos' iCloud service.
This also acts as the shared library if I am not the owner of the library.
"""The 'Shared Photos' iCloud service.
Provides access to shared photo libraries, such as shared albums or libraries
that the user does not own. This is distinct from the regular PhotosService,
which provides access to the user's primary photo library.

Copilot uses AI. Check for mistakes.
"""

def __init__(self, service_root, session, params):
self.session = session
self.params = dict(params)
self._service_root = service_root

self._libraries = None

self.params.update({"remapEnums": True, "getCurrentSyncToken": True})

self._service_endpoint = f"{self._service_root}/database/1/com.apple.photos.cloud/production/shared"

self._libraries = None
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self._libraries is assigned None twice (lines 288 and 294). The duplicate assignment on line 294 should be removed.

Suggested change
self._libraries = None

Copilot uses AI. Check for mistakes.

self._photo_assets = {}

try:
# url = f"{self._service_endpoint}/zones/list"
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented-out code should be removed. If the alternative URL is needed for reference, it should be documented in a comment explaining why the current URL is used instead.

Suggested change
# url = f"{self._service_endpoint}/zones/list"

Copilot uses AI. Check for mistakes.
url = f"{self._service_root}/database/1/com.apple.photos.cloud/production/shared/zones/list"
request = self.session.post(
url,
data="{}",
headers={"Content-type": "text/plain"},
)
response = request.json()
zones = response["zones"]
except Exception as e:
LOGGER.error(f"library exception: {str(e)}")

# The call to `/records/query` requires the `ownerRecordName` to be provided, which is known only after obtaining it from the API.

if not zones:
Comment on lines +298 to +313
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable zones may not be defined if the exception occurs on line 308. This will cause a NameError when checking if not zones: on line 313. Initialize zones = [] before the try block to ensure the variable is always defined.

Copilot uses AI. Check for mistakes.
return
Comment on lines +310 to +314
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The super().__init__() call only happens if zones is truthy (line 315). This means if there are no zones or an exception occurs, the SharedPhotosService instance will not be properly initialized as a PhotoLibrary, lacking essential attributes like self.zone_id and self._albums. This could cause AttributeError when accessing properties inherited from PhotoLibrary. Consider how to handle the no-zones case - either raise an exception or initialize with a default/empty zone_id.

Suggested change
# The call to `/records/query` requires the `ownerRecordName` to be provided, which is known only after obtaining it from the API.
if not zones:
return
raise ICloudPyServiceNotActivatedException(
"Unable to fetch shared photo zones: " + str(e), None
)
# The call to `/records/query` requires the `ownerRecordName` to be provided, which is known only after obtaining it from the API.
if not zones:
raise ICloudPyServiceNotActivatedException(
"No shared photo zones found for this account.", None
)

Copilot uses AI. Check for mistakes.
super().__init__(service=self, zone_id=zones[0]["zoneID"])

@property
def libraries(self):
if not self._libraries:
try:
url = f"{self._service_endpoint}/zones/list"
request = self.session.post(
url,
data="{}",
headers={"Content-type": "text/plain"},
)
response = request.json()
zones = response["zones"]
except Exception as e:
LOGGER.error(f"library exception: {str(e)}")

libraries = {}
for zone in zones:
if not zone.get("deleted"):
zone_name = zone["zoneID"]["zoneName"]
libraries[zone_name] = PhotoLibrary(self, zone_id=zone["zoneID"])
# obj_type='CPLAssetByAssetDateWithoutHiddenOrDeleted',
# list_type="CPLAssetAndMasterByAssetDateWithoutHiddenOrDeleted",
# direction="ASCENDING", query_filter=None,
# zone_id=zone['zoneID'])
return libraries
Comment on lines +318 to +341
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The libraries property has multiple issues:

  1. If an exception occurs on line 329, zones may not be defined, causing a NameError when iterating with for zone in zones: on line 333. Initialize zones = [] before the try block.
  2. The method doesn't assign self._libraries = libraries before returning (compare with PhotosService.libraries line 273), which means subsequent calls won't use the cached value.
  3. Missing explicit return statement - should return libraries at the end.

Copilot uses AI. Check for mistakes.
Comment on lines +277 to +341
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new SharedPhotosService class lacks test coverage. Following the project's testing pattern (as seen in tests/test_photos.py for PhotosService), tests should be added to verify:

  1. Service initialization with correct params
  2. Zone ID handling for shared photos
  3. The libraries property
  4. Exception handling in __init__ when zones cannot be fetched
  5. The service endpoint structure for shared photos

Add fixtures to tests/const_photos.py for shared photo zones/list responses and update tests/__init__.py::ICloudPySessionMock.request() to handle the shared photo URLs.

Copilot generated this review using guidance from repository custom instructions.

class PhotoAlbum:
"""A photo album."""
Expand Down