Skip to content

Commit f363a78

Browse files
refactor: extract service dependencies from PhotoLibrary and PhotoAlbum constructors
- Update PhotoLibrary constructor to accept service_endpoint, params, session directly - Update PhotoAlbum constructor to accept params, session directly instead of service - Remove service coupling by passing dependencies explicitly - Update all callers to provide dependencies directly - Fix PhotosService inheritance to properly initialize parent constructor - Update base.py to use direct library properties (params, session) instead of service.* - All tests pass and mypy strict type checking succeeds Benefits: - Better testability through dependency injection - Cleaner architecture following dependency inversion principle - Consistent with PhotoAsset.download() refactoring pattern - Reduced coupling between service layers 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 4c7990d commit f363a78

File tree

2 files changed

+28
-26
lines changed

2 files changed

+28
-26
lines changed

src/icloudpd/base.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -841,7 +841,7 @@ def delete_photo(
841841
logger.debug("Deleting %s in iCloud...", clean_filename_local)
842842
url = (
843843
f"{library_object.service_endpoint}/records/modify?"
844-
f"{urllib.parse.urlencode(library_object.service.params)}"
844+
f"{urllib.parse.urlencode(library_object.params)}"
845845
)
846846
post_data = json.dumps(
847847
{
@@ -861,9 +861,7 @@ def delete_photo(
861861
"zoneID": library_object.zone_id,
862862
}
863863
)
864-
library_object.service.session.post(
865-
url, data=post_data, headers={"Content-type": "application/json"}
866-
)
864+
library_object.session.post(url, data=post_data, headers={"Content-type": "application/json"})
867865
logger.info("Deleted %s in iCloud", clean_filename_local)
868866

869867

src/pyicloud_ipd/services/photos.py

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -271,20 +271,21 @@ class PhotoLibrary:
271271
},
272272
}
273273

274-
def __init__(self, service: "PhotosService", zone_id: Dict[str, Any], library_type: str):
275-
self.service = service
274+
def __init__(self, service_endpoint: str, params: Dict[str, Any], session: Session, zone_id: Dict[str, Any], library_type: str):
275+
self.service_endpoint = service_endpoint
276+
self.params = params
277+
self.session = session
276278
self.zone_id = zone_id
277279
self.library_type = library_type
278-
self.service_endpoint = self.service.get_service_endpoint(library_type)
279280

280281
url = ('%s/records/query?%s' %
281-
(self.service_endpoint, urlencode(self.service.params)))
282+
(self.service_endpoint, urlencode(self.params)))
282283
json_data = json.dumps({
283284
"query": {"recordType":"CheckIndexingState"},
284285
"zoneID": self.zone_id,
285286
})
286287

287-
request = self.service.session.post(
288+
request = self.session.post(
288289
url,
289290
data=json_data,
290291
headers={'Content-type': 'text/plain'}
@@ -298,7 +299,7 @@ def __init__(self, service: "PhotosService", zone_id: Dict[str, Any], library_ty
298299
@property
299300
def albums(self) -> Dict[str, "PhotoAlbum"]:
300301
albums = {
301-
name: PhotoAlbum(self.service, self.service_endpoint, name, zone_id=self.zone_id, **props) # type: ignore[arg-type] # dynamically builing params
302+
name: PhotoAlbum(self.params, self.session, self.service_endpoint, name, zone_id=self.zone_id, **props) # type: ignore[arg-type] # dynamically builing params
302303
for (name, props) in self.SMART_FOLDERS.items()
303304
}
304305

@@ -324,7 +325,7 @@ def albums(self) -> Dict[str, "PhotoAlbum"]:
324325
}
325326
}]
326327

327-
album = PhotoAlbum(self.service, self.service_endpoint, folder_name,
328+
album = PhotoAlbum(self.params, self.session, self.service_endpoint, folder_name,
328329
'CPLContainerRelationLiveByAssetDate',
329330
folder_obj_type, query_filter,
330331
zone_id=self.zone_id)
@@ -336,13 +337,13 @@ def _fetch_folders(self) -> Sequence[Dict[str, Any]]:
336337
if self.library_type == "shared":
337338
return []
338339
url = ('%s/records/query?%s' %
339-
(self.service_endpoint, urlencode(self.service.params)))
340+
(self.service_endpoint, urlencode(self.params)))
340341
json_data = json.dumps({
341342
"query": {"recordType":"CPLAlbumByPositionLive"},
342343
"zoneID": self.zone_id,
343344
})
344345

345-
request = self.service.session.post(
346+
request = self.session.post(
346347
url,
347348
data=json_data,
348349
headers={'Content-type': 'text/plain'}
@@ -353,11 +354,11 @@ def _fetch_folders(self) -> Sequence[Dict[str, Any]]:
353354

354355
@property
355356
def all(self) -> "PhotoAlbum":
356-
return PhotoAlbum(self.service, self.service_endpoint, "", zone_id=self.zone_id, **self.WHOLE_COLLECTION) # type: ignore[arg-type] # dynamically builing params
357+
return PhotoAlbum(self.params, self.session, self.service_endpoint, "", zone_id=self.zone_id, **self.WHOLE_COLLECTION) # type: ignore[arg-type] # dynamically builing params
357358

358359
@property
359360
def recently_deleted(self) -> "PhotoAlbum":
360-
return PhotoAlbum(self.service, self.service_endpoint, "", zone_id=self.zone_id, **self.RECENTLY_DELETED) # type: ignore[arg-type] # dynamically builing params
361+
return PhotoAlbum(self.params, self.session, self.service_endpoint, "", zone_id=self.zone_id, **self.RECENTLY_DELETED) # type: ignore[arg-type] # dynamically builing params
361362

362363
class PhotosService(PhotoLibrary):
363364
"""The 'Photos' iCloud service.
@@ -376,11 +377,15 @@ def __init__(
376377
self._private_libraries: Dict[str, PhotoLibrary] | None = None
377378
self._shared_libraries: Dict[str, PhotoLibrary] | None = None
378379

379-
380380
self.params.update({
381381
'remapEnums': True,
382382
'getCurrentSyncToken': True
383383
})
384+
385+
# Initialize as primary library
386+
service_endpoint = self.get_service_endpoint("private")
387+
zone_id = {'zoneName': 'PrimarySync'}
388+
super().__init__(service_endpoint, self.params, self.session, zone_id, "private")
384389

385390
# TODO: Does syncToken ever change?
386391
# self.params.update({
@@ -390,9 +395,6 @@ def __init__(
390395

391396
# self._photo_assets = {}
392397

393-
super(PhotosService, self).__init__(
394-
service=self, zone_id={'zoneName': 'PrimarySync'}, library_type="private")
395-
396398
@property
397399
def private_libraries(self) -> Dict[str, PhotoLibrary]:
398400
if not self._private_libraries:
@@ -422,8 +424,9 @@ def _fetch_libraries(self, library_type: str) -> Dict[str, PhotoLibrary]:
422424
for zone in response['zones']:
423425
if not zone.get('deleted'):
424426
zone_name = zone['zoneID']['zoneName']
427+
service_endpoint = self.get_service_endpoint(library_type)
425428
libraries[zone_name] = PhotoLibrary(
426-
self, zone_id=zone['zoneID'], library_type=library_type)
429+
service_endpoint, self.params, self.session, zone_id=zone['zoneID'], library_type=library_type)
427430
# obj_type='CPLAssetByAssetDateWithoutHiddenOrDeleted',
428431
# list_type="CPLAssetAndMasterByAssetDateWithoutHiddenOrDeleted",
429432
# direction="ASCENDING", query_filter=None,
@@ -439,10 +442,11 @@ def get_service_endpoint(self, library_type: str) -> str:
439442

440443
class PhotoAlbum:
441444

442-
def __init__(self, service:PhotosService, service_endpoint: str, name: str, list_type: str, obj_type: str,
445+
def __init__(self, params: Dict[str, Any], session: Session, service_endpoint: str, name: str, list_type: str, obj_type: str,
443446
query_filter:Sequence[Dict[str, Any]] | None=None, page_size:int=100, zone_id:Dict[str, Any] | None=None):
444447
self.name = name
445-
self.service = service
448+
self.params = params
449+
self.session = session
446450
self.service_endpoint = service_endpoint
447451
self.list_type = list_type
448452
self.obj_type = obj_type
@@ -465,8 +469,8 @@ def __iter__(self) -> Generator["PhotoAsset", Any, None]:
465469
def __len__(self) -> int:
466470
url = ('%s/internal/records/query/batch?%s' %
467471
(self.service_endpoint,
468-
urlencode(self.service.params)))
469-
request = self.service.session.post(
472+
urlencode(self.params)))
473+
request = self.session.post(
470474
url,
471475
data=json.dumps(self._count_query_gen(self.obj_type)),
472476
headers={'Content-type': 'text/plain'}
@@ -480,8 +484,8 @@ def __len__(self) -> int:
480484
# can mock it to test session errors.
481485
def photos_request(self) -> Response:
482486
url = ('%s/records/query?' % self.service_endpoint) + \
483-
urlencode(self.service.params)
484-
return self.service.session.post(
487+
urlencode(self.params)
488+
return self.session.post(
485489
url,
486490
data=json.dumps(self._list_query_gen(
487491
self.offset, self.list_type,

0 commit comments

Comments
 (0)