|
| 1 | +# Dependency Injection and Functional Composition Refactoring |
| 2 | + |
| 3 | +## Summary |
| 4 | +This refactoring series implements dependency injection patterns and functional composition throughout the icloud_photos_downloader codebase, removing tight coupling between service layers and improving testability. |
| 5 | + |
| 6 | +## Completed Refactorings |
| 7 | + |
| 8 | +### 1. Extract Fingerprint Hash Logic from calculate_filename() |
| 9 | +**Commit**: `caf2a92 - Extract fingerprint hash logic from calculate_filename() into separate fallback function` |
| 10 | + |
| 11 | +**Changes**: |
| 12 | +- Made `calculate_filename()` return `Optional[str]` (None when filenameEnc missing) |
| 13 | +- Created `generate_fingerprint_filename()` for fallback logic |
| 14 | +- Created `bind_filename_with_fallback()` to handle None returns |
| 15 | +- Updated all callers to use binding pattern |
| 16 | + |
| 17 | +**Pattern**: Optional/Maybe pattern with fallback composition |
| 18 | + |
| 19 | +### 2. Extract raw_policy from PyiCloudService |
| 20 | +**Commit**: `ad452c3 - Extract raw_policy from PyiCloudService using dependency injection pattern` |
| 21 | + |
| 22 | +**Changes**: |
| 23 | +- Removed `raw_policy` parameter from PyiCloudService and PhotosService constructors |
| 24 | +- Created `apply_raw_policy()` standalone function |
| 25 | +- Added `versions_with_raw_policy()` method to PhotoAsset |
| 26 | +- Updated all callers to pass `raw_policy` explicitly |
| 27 | +- Updated download_builder signature to include raw_policy parameter |
| 28 | + |
| 29 | +**Pattern**: Dependency injection - pass parameters explicitly instead of storing in service |
| 30 | + |
| 31 | +### 3. Implement fromMaybe Function (Haskell-style) |
| 32 | +**Commit**: `a6f362c - Implement fromMaybe function and replace bind_filename_with_fallback with functional composition` |
| 33 | + |
| 34 | +**Changes**: |
| 35 | +- Created `foundation.core.optional.fromMaybe()` function similar to Haskell's fromMaybe |
| 36 | +- Replaced `bind_filename_with_fallback` with `filename_with_fallback` using fromMaybe |
| 37 | +- Applied functional composition pattern: `fromMaybe(default)(maybe_value)` |
| 38 | + |
| 39 | +**Pattern**: Functional composition with Higher-Order Functions |
| 40 | + |
| 41 | +### 4. Extract Download Function from PhotoAsset |
| 42 | +**Commit**: `0344d94 - refactor: use base Session class instead of PyiCloudSession in PhotoAsset.download` |
| 43 | + |
| 44 | +**Changes**: |
| 45 | +- Created standalone `download_asset()` function taking session parameter |
| 46 | +- Updated `PhotoAsset.download()` to accept session parameter |
| 47 | +- Removed service dependency from PhotoAsset constructor |
| 48 | +- Used base `Session` class instead of `PyiCloudSession` for better abstraction |
| 49 | +- Fixed all test mock functions to match new signatures |
| 50 | + |
| 51 | +**Pattern**: Dependency injection + Interface Segregation (using base Session type) |
| 52 | + |
| 53 | +### 5. Extract Service Dependencies from PhotoLibrary and PhotoAlbum |
| 54 | +**Commit**: `26c96b2 - refactor: extract service dependencies from PhotoLibrary and PhotoAlbum constructors` |
| 55 | + |
| 56 | +**Changes**: |
| 57 | +- Updated PhotoLibrary constructor: `(service, zone_id, library_type)` → `(service_endpoint, params, session, zone_id, library_type)` |
| 58 | +- Updated PhotoAlbum constructor: `(service, service_endpoint, ...)` → `(params, session, service_endpoint, ...)` |
| 59 | +- Updated all callers to pass dependencies explicitly |
| 60 | +- Fixed PhotosService inheritance with proper parent constructor |
| 61 | +- Updated base.py to use direct properties instead of `service.*` |
| 62 | + |
| 63 | +**Pattern**: Constructor Dependency Injection |
| 64 | + |
| 65 | +## Key Patterns Implemented |
| 66 | + |
| 67 | +### 1. **Dependency Injection** |
| 68 | +- **Before**: Classes stored entire service objects and accessed nested properties |
| 69 | +- **After**: Classes receive only the specific dependencies they need |
| 70 | +- **Benefits**: Better testability, reduced coupling, clearer dependencies |
| 71 | + |
| 72 | +### 2. **Functional Composition** |
| 73 | +- **Before**: Imperative if/else logic for handling optional values |
| 74 | +- **After**: Functional composition with higher-order functions like `fromMaybe` |
| 75 | +- **Benefits**: More declarative code, better composability |
| 76 | + |
| 77 | +### 3. **Optional/Maybe Pattern** |
| 78 | +- **Before**: Methods threw exceptions or returned default values internally |
| 79 | +- **After**: Methods return `Optional[T]` and callers handle None explicitly |
| 80 | +- **Benefits**: Explicit error handling, no hidden defaults |
| 81 | + |
| 82 | +### 4. **Interface Segregation** |
| 83 | +- **Before**: Depended on concrete `PyiCloudSession` class |
| 84 | +- **After**: Depend on base `Session` interface from requests library |
| 85 | +- **Benefits**: More flexible, easier to mock in tests |
| 86 | + |
| 87 | +## Architecture Impact |
| 88 | + |
| 89 | +### Service Layer Decoupling |
| 90 | +``` |
| 91 | +Before: PyiCloudService → PhotosService → PhotoLibrary → PhotoAlbum → PhotoAsset |
| 92 | + (Tight coupling - each layer stores references to parent services) |
| 93 | +
|
| 94 | +After: PyiCloudService → PhotosService → PhotoLibrary → PhotoAlbum → PhotoAsset |
| 95 | + (Loose coupling - dependencies passed explicitly as needed) |
| 96 | +``` |
| 97 | + |
| 98 | +### Parameter Flow |
| 99 | +``` |
| 100 | +Before: raw_policy stored in PyiCloudService, accessed via service.raw_policy |
| 101 | +After: raw_policy passed explicitly: download_builder(raw_policy, ...) → photo.versions_with_raw_policy(raw_policy) |
| 102 | +
|
| 103 | +Before: session accessed via PhotoAsset.service.session |
| 104 | +After: session passed explicitly: photo.download(session, url, start) |
| 105 | +
|
| 106 | +Before: PhotoLibrary accesses service.params, service.session, service.get_service_endpoint() |
| 107 | +After: PhotoLibrary receives params, session, service_endpoint directly |
| 108 | +``` |
| 109 | + |
| 110 | +## Quality Metrics |
| 111 | + |
| 112 | +- **Tests**: All 203 tests pass (200 passed, 3 skipped) |
| 113 | +- **Type Safety**: mypy strict mode passes on 73 files (src + tests) |
| 114 | +- **Code Quality**: ruff linting passes with no issues |
| 115 | +- **Formatting**: ruff format applied consistently |
| 116 | + |
| 117 | +## File Changes Summary |
| 118 | + |
| 119 | +### Core Service Files |
| 120 | +- `src/pyicloud_ipd/services/photos.py` - Major refactoring of PhotoAsset, PhotoLibrary, PhotoAlbum, PhotosService |
| 121 | +- `src/pyicloud_ipd/base.py` - Updated PyiCloudService and download_builder |
| 122 | +- `src/icloudpd/base.py` - Updated callers to use new dependency injection patterns |
| 123 | +- `src/icloudpd/download.py` - Updated to pass session explicitly to PhotoAsset.download() |
| 124 | + |
| 125 | +### Foundation Layer |
| 126 | +- `src/foundation/core/optional/__init__.py` - Added fromMaybe function for functional composition |
| 127 | + |
| 128 | +### Test Files |
| 129 | +- `tests/test_download_photos.py` - Updated mock functions for new PhotoAsset.download signature |
| 130 | +- `tests/test_download_photos_id.py` - Updated mock functions for new PhotoAsset.download signature |
| 131 | + |
| 132 | +## Parameters Passed to PhotosService |
| 133 | +PhotosService receives params from PyiCloudService (initially empty dict) and adds: |
| 134 | +```python |
| 135 | +{ |
| 136 | + 'remapEnums': True, # Tells iCloud API to remap enumeration values |
| 137 | + 'getCurrentSyncToken': True # Requests current synchronization token |
| 138 | +} |
| 139 | +``` |
| 140 | + |
| 141 | +These params are used in `urlencode(params)` for building iCloud API query strings throughout the photo library operations. |
| 142 | + |
| 143 | +## Migration Guide for Future Development |
| 144 | + |
| 145 | +### Adding New Dependencies |
| 146 | +1. **Identify the minimal dependency needed** (not entire service objects) |
| 147 | +2. **Pass dependencies through constructor parameters** |
| 148 | +3. **Update all callers** to provide the dependency explicitly |
| 149 | +4. **Write tests** that can mock the dependency easily |
| 150 | + |
| 151 | +### Following the Patterns |
| 152 | +1. **Use Optional[T] return types** for methods that might not find results |
| 153 | +2. **Use fromMaybe()** for providing defaults to Optional values |
| 154 | +3. **Pass session parameters explicitly** instead of storing in services |
| 155 | +4. **Use base interface types** (like Session) instead of concrete implementations |
| 156 | + |
| 157 | +This refactoring establishes a foundation for more maintainable, testable, and loosely-coupled code following functional programming and dependency injection principles. |
0 commit comments