Refactor dependency injection functional composition#1236
Merged
AndreyNikiforov merged 14 commits intoicloud-photos-downloader:masterfrom Aug 30, 2025
Conversation
…on to caller - Remove filename_override property from AssetVersion class - Update disambiguate_filenames() to return tuple with override mappings - Modify PhotoAsset.calculate_version_filename() to accept optional override parameter - Update all calling code to handle new disambiguation API - Maintain thread safety and clear separation of concerns - All tests pass, bug icloud-photos-downloader#926 fix preserved 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Move ITEM_TYPE_EXTENSIONS and VERSION_FILENAME_SUFFIX_LOOKUP constants to asset_version.py - Create static calculate_version_filename() function with all required parameters - Update PhotoAsset.calculate_version_filename() to use static function as wrapper - Replace method calls with static function in base.py and autodelete.py for better performance - Maintain backward compatibility with PhotoAsset method for existing code - Enable direct static calls when all parameters are available 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Remove lp_filename_generator parameter from PhotosService constructor and pass it explicitly to functions that need it. This improves separation of concerns by removing presentation layer dependencies from the service layer. Changes: - Remove lp_filename_generator from PhotosService constructor and storage - Update function signatures to accept lp_filename_generator parameter: - disambiguate_filenames() in utils.py - autodelete_photos() in autodelete.py - download_builder() in base.py - PhotoAsset.calculate_version_filename() method - Update all calling code to pass lp_filename_generator explicitly - Fix test mocks to match updated method signatures - Fix error logging in download.py to use static calculate_version_filename 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Remove lp_filename_generator parameter from PyiCloudService constructor and all related calling code. This completes the dependency injection refactoring by removing all presentation layer dependencies from the service layer. Changes: - Remove lp_filename_generator parameter from PyiCloudService constructor - Update authenticator() function to no longer accept lp_filename_generator - Fix all calling code in base.py, authentication.py, cmdline.py and tests - Remove unused import in test_authentication.py The lp_filename_generator is now only passed directly to specific functions that need it, improving separation of concerns and following better dependency injection principles. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…architecture - Extract filename_cleaner from PyiCloudService to follow dependency injection pattern - Add build_filename_cleaner factory that respects user's keep_unicode_in_filenames preference - Use clean_filename directly in PhotoAsset.calculate_filename for filesystem safety - Pass filename_cleaner parameter explicitly to download_builder for unicode handling - Maintain two-tier filename cleaning: clean_filename (always) + optional unicode removal - Preserve backward compatibility through PhotoAsset.filename property using identity function - All 91 download tests pass including Chinese unicode preservation tests 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Remove file_match_policy as a constructor parameter from PyiCloudService and PhotosService, implementing dependency injection pattern instead. The parameter is now passed explicitly to functions that need it. Key changes: - Removed file_match_policy from PyiCloudService and PhotosService constructors - Updated authenticator() to not require file_match_policy parameter - Modified message functions (skip_created_*_message, asset_type_skip_message, delete_photo*) to accept optional file_match_policy and filename_cleaner parameters - Updated download_media() to accept file_match_policy and filename_cleaner for proper error messages - PhotoAsset.filename property now uses default FileMatchPolicy.NAME_SIZE_DEDUP_WITH_SUFFIX for backward compatibility - Updated all function calls to pass file_match_policy and filename_cleaner explicitly where needed - Fixed test mock expectations to match new function signatures - Added proper type annotations for all new parameters This refactoring achieves better separation of concerns by removing presentation-layer dependencies from the service layer while maintaining full backward compatibility. All 220 tests pass, including NAME_ID7 tests that previously failed due to incorrect filename handling in log messages and error outputs. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…o separate function Extract the file match policy transformation logic from PhotoAsset.calculate_filename() into a separate apply_file_match_policy() function, implementing functional composition pattern. Key changes: - Created apply_file_match_policy(file_match_policy, asset_id) function that returns a filename transformer - Updated calculate_filename() to be policy-agnostic, only handling filename parsing and cleaning - Removed file_match_policy parameter from calculate_filename() - Updated all callers to use composition pattern: apply_file_match_policy(policy, id)(calculate_filename(cleaner)) - Updated 15+ call sites across base.py, download.py, and photos.py - Maintained backward compatibility through composition in the filename property - Added proper type annotations for all new functions Benefits: - Better separation of concerns: filename parsing vs policy application - Improved composability: policies can be easily composed with filename calculation - Enhanced extensibility: new policies can be added by extending apply_file_match_policy - Maintains type safety and backward compatibility All tests pass including NAME_ID7 functionality and mypy strict type checking. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
… separate function Extract the filename cleaning logic from PhotoAsset.calculate_filename() into a separate apply_filename_cleaner() function, completing the functional composition pattern for filename processing. Key changes: - Created apply_filename_cleaner(filename_cleaner) function that returns a filename cleaning transformer - Updated calculate_filename() to be parameter-free, returning only raw parsed filenames - Removed filename_cleaner parameter from calculate_filename() - Updated all callers to use three-stage composition: 1. raw_filename = photo.calculate_filename() 2. cleaned_filename = apply_filename_cleaner(cleaner)(raw_filename) 3. final_filename = apply_file_match_policy(policy, id)(cleaned_filename) - Updated 15+ call sites across base.py, download.py, and photos.py - Maintained backward compatibility through composition in the filename property Architecture achieved: - Complete separation of concerns: raw parsing, cleaning, and policy application - Pure functional composition with composable transformations - Maximum extensibility for new cleaning strategies and policies - Type safety maintained throughout Benefits: - Each transformation stage is now a pure, composable function - Easy to add new cleaning or policy strategies - Clear separation between parsing, cleaning, and policy application - Improved testability and maintainability All tests pass including NAME_ID7 functionality and mypy strict type checking. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…e fallback function This refactoring: 1. Extracts fingerprint-based filename generation into generate_fingerprint_filename() 2. Makes calculate_filename() return Optional[str] (None when filenameEnc is missing) 3. Creates bind_filename_with_fallback() to handle Optional returns with fingerprint fallback 4. Updates all 12 callers to use binding pattern: fallback_binder(photo.calculate_filename) The functional composition pattern now handles all cases: - Normal filename extraction from filenameEnc - Fingerprint-based fallback when filenameEnc is unavailable - Filename cleaning transformations - File match policy transformations 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…ttern This refactoring removes raw_policy from the service layer constructors and applies dependency injection: 1. Remove raw_policy parameter from PyiCloudService and PhotosService constructors 2. Extract raw policy logic into apply_raw_policy() function 3. Add versions_with_raw_policy() method to PhotoAsset for explicit policy application 4. Update autodelete_photos() and download_builder() to accept raw_policy parameter 5. Update all callers to pass user_config.align_raw explicitly 6. Fix function parameter order to maintain backward compatibility The functional composition pattern now handles raw policy transformations explicitly rather than through service state, improving testability and separation of concerns. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…with functional composition This refactoring introduces a generic fromMaybe function following Haskell's pattern and simplifies filename fallback logic: 1. Add fromMaybe(default) -> (Maybe a -> a) function to foundation.core.optional 2. Replace bind_filename_with_fallback with filename_with_fallback using fromMaybe 3. Simplify all callers from complex binding pattern to direct function composition: - Before: fallback_binder(photo.calculate_filename) - After: extract_with_fallback(photo.calculate_filename()) The functional composition is now more idiomatic and leverages standard Maybe monad patterns. The fromMaybe function provides a reusable utility for extracting values from Optional types with fallbacks. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…sset.download - Extract download functionality into standalone download_asset() function - Update PhotoAsset.download() to accept session parameter using base Session type - Remove service dependency from PhotoAsset constructor - Fix all test mock functions to match new method signature - All tests pass and mypy strict type checking succeeds 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…um 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>
- Document complete dependency injection and functional composition refactoring series - Detail all 5 major refactoring commits with patterns and benefits - Explain architecture impact and service layer decoupling - Provide migration guide for future development following established patterns - Include parameter flow documentation and quality metrics - Capture knowledge for maintaining consistent architectural patterns 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
AndreyNikiforov
merged commit d45c4b9
into
icloud-photos-downloader:master
398 checks passed
AndreyNikiforov
deleted the
refactor/dependency-injection-functional-composition
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.