centralize filepath creation

[Kimblebee]

Extracted the filename and output directory generation logic from the LocalMediaRepository, PostCaptureViewModel, and CameraCoreUtil into a new FilePathGenerator interface.

This change makes the project friendlier to forks by allowing developers to provide their own implementation of FilePathGenerator to customize file naming and save locations without modifying the core application logic.

Key changes:

• Created FilePathGenerator interface in core:common.
• Implemented JcaFilePathGenerator in the app module as the default implementation.
• Injected FilePathGenerator into LocalMediaRepository and CameraXCameraSystem.
• Updated tests to provide a FakeFilePathGenerator and adjusted mock stubs.

[Kimblebee]

/gemini summary

[Kimblebee]

/gemini review

[gemini-code-assist]

Summary of Changes

This pull request refactors the application's media file path and name generation logic by extracting it into a dedicated FilePathGenerator interface. This significantly improves the architecture by centralizing a common concern, making the codebase more modular, easier to test, and more adaptable for future customizations or forks. The change abstracts away the specifics of how filenames and paths are determined, allowing for flexible implementations.

Highlights

• Centralized File Path Generation: Introduced a new FilePathGenerator interface in core:common to centralize the logic for generating filenames and output directories for captured media. This replaces scattered logic previously found in LocalMediaRepository, PostCaptureViewModel, and CameraCoreUtil.
• Default Implementation and Dependency Injection: A default implementation, JcaFilePathGenerator, has been added to the app module. The FilePathGenerator is now injected into LocalMediaRepository and CameraXCameraSystem, promoting better modularity and testability.
• Improved Extensibility: This change makes the project more friendly to forks, allowing developers to provide their own custom FilePathGenerator implementations to customize file naming and save locations without needing to modify core application logic.
• Test Updates: Tests have been updated to utilize a FakeFilePathGenerator for consistent and isolated testing of components that rely on file path generation.

Changelog

• app/src/main/java/com/google/jetpackcamera/AppModule.kt
  • Added imports for DefaultFilePathGenerator and FilePathGenerator.
  • Provided JcaFilePathGenerator as the default implementation for FilePathGenerator.
• app/src/main/java/com/google/jetpackcamera/JcaFilePathGenerator.kt
  • New file: Implements the FilePathGenerator interface, providing default logic for generating image and video filenames and their respective output paths.
• core/camera/src/androidTest/java/com/google/jetpackcamera/core/camera/CameraXCameraSystemTest.kt
  • Imported FakeFilePathGenerator.
  • Injected FakeFilePathGenerator into the CameraXCameraSystem constructor for testing purposes.
• core/camera/src/main/java/com/google/jetpackcamera/core/camera/CameraCoreUtil.kt
  • Removed the getDefaultVideoSaveLocation() utility function, as its logic is now handled by FilePathGenerator.
• core/camera/src/main/java/com/google/jetpackcamera/core/camera/CameraSession.kt
  • Removed imports for old file path utility functions.
  • Modified getPendingRecording and runVideoRecording functions to accept and utilize FilePathGenerator for video filename and path determination.
• core/camera/src/main/java/com/google/jetpackcamera/core/camera/CameraSessionContext.kt
  • Added filePathGenerator as a parameter to the CameraSessionContext data class.
• core/camera/src/main/java/com/google/jetpackcamera/core/camera/CameraXCameraSystem.kt
  • Removed imports for old file path utility functions and date formatting.
  • Injected FilePathGenerator into the CameraXCameraSystem constructor.
  • Updated image capture logic to use filePathGenerator for generating filenames and relative paths.
• core/common/src/main/java/com/google/jetpackcamera/core/common/CommonModule.kt
  • Added @DefaultFilePathGenerator annotation for Hilt dependency injection.
• core/common/src/main/java/com/google/jetpackcamera/core/common/FakeFilePathGenerator.kt
  • New file: Provides a test-specific implementation of FilePathGenerator for mock filename and path generation.
• core/common/src/main/java/com/google/jetpackcamera/core/common/FilePathGenerator.kt
  • New file: Defined the FilePathGenerator interface with methods for generating image/video filenames and providing relative/absolute output paths.
• data/media/build.gradle.kts
  • Changed implementation(project(":core:camera")) to testImplementation(project(":core:common")).
• data/media/src/main/kotlin/com/google/jetpackcamera/data/media/FakeMediaRepository.kt
  • Updated the saveToMediaStore function signature to accept a nullable outputFilename.
• data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt
  • Imported DefaultFilePathGenerator and FilePathGenerator.
  • Injected FilePathGenerator into the LocalMediaRepository constructor.
  • Refactored media deletion logic into deleteCachedMedia and deleteContentMedia.
  • Updated saveToMediaStore to use filePathGenerator for determining filenames and output directories.
  • Modified copyToMediaStore parameters to align with the new FilePathGenerator usage.
• data/media/src/main/kotlin/com/google/jetpackcamera/data/media/MediaRepository.kt
  • Updated the saveToMediaStore function signature to accept a nullable outputFilename.
• data/media/src/test/java/com/google/jetpackcamera/media/LocalMediaRepositoryTest.kt
  • Imported FakeFilePathGenerator.
  • Injected FakeFilePathGenerator into the LocalMediaRepository constructor for testing.
• feature/postcapture/src/main/java/com/google/jetpackcamera/feature/postcapture/PostCaptureViewModel.kt
  • Removed the createFilename utility function and related date/locale imports.
  • Updated saveToMediaStore calls to pass null for the filename, allowing LocalMediaRepository to use the injected FilePathGenerator.
• feature/postcapture/src/test/java/com/google/jetpackcamera/feature/postcapture/PostCaptureViewModelTest.kt
  • Imported isNull for Mockito argument matching.
  • Updated saveToMediaStore mock stubs to expect null for the filename argument.

Activity

• Kimblebee requested a Gemini summary.
• Kimblebee requested a Gemini review.

[gemini-code-assist]

Code Review

This pull request does a great job of centralizing the file path and name generation logic into a new FilePathGenerator interface, which improves modularity and makes the system more extensible. The dependency is correctly injected where needed, and the tests have been updated with a fake implementation, which is a solid approach.

I've found one high-severity issue where a previously used suffix for video filenames is now missing, which could impact debugging. I've also left a few medium-severity comments related to improving documentation and adhering to Kotlin's idiomatic style, as suggested by the repository's style guide.

[Kimblebee]

/gemini review

[gemini-code-assist]

Code Review

This pull request does a great job of centralizing the filepath creation logic by introducing the FilePathGenerator interface. This is a solid architectural improvement that enhances modularity and makes the code easier to customize. The changes are well-integrated across the various modules, and the tests have been updated accordingly.

I've identified a critical issue related to a potential NullPointerException when generating video paths on older Android versions. Additionally, there are opportunities to improve code quality by adhering to the DRY principle in the new generator classes and simplifying some of the new Dagger provider code. Addressing these points will make the implementation more robust and maintainable.

[Kimblebee]

/gemini review

[gemini-code-assist]

Code Review

This pull request effectively centralizes the filepath and filename generation logic by introducing a FilePathGenerator interface. This is a solid architectural improvement that removes duplicated code from various classes like LocalMediaRepository and PostCaptureViewModel, enhancing modularity and making the system more extensible for forks. The use of dependency injection for the new service and the addition of a FakeFilePathGenerator for tests are well-implemented. My review includes one suggestion to further reduce code duplication between the concrete FilePathGenerator implementations to improve maintainability.

[Kimblebee]

/gemini review

[gemini-code-assist]

Code Review

This pull request is an excellent refactoring that successfully centralizes the file path and directory generation logic into a new FilePathGenerator interface. This improves modularity and makes the system more extensible, which aligns perfectly with the stated goal of making the project friendlier to forks. The introduction of a FakeFilePathGenerator for testing is also a great addition that follows best practices.

I have two points of feedback:

1. A high-severity concern regarding a change in the default SaveMode in AppModule.kt, which alters core application behavior and seems unrelated to the PR's main objective.
2. A medium-severity suggestion to fix a minor typo in the KDoc for the new FilePathGenerator.kt interface to improve documentation clarity.

[Kimblebee]

/gemini review

[gemini-code-assist]

Code Review

This pull request does a great job of centralizing the filepath and filename generation logic by introducing a new FilePathGenerator interface. This is a solid refactoring that improves modularity, testability, and makes it easier for forks to customize file handling, adhering to the project's architectural principles.

The implementation is well-executed across the board, from the new interface and its default implementation to the updates in the camera and data layers. The use of a FakeFilePathGenerator in tests is also a great example of following the testing guidelines.

I have one minor suggestion in JcaFilePathGenerator.kt to reduce a small amount of code duplication, in line with the DRY principle mentioned in the style guide.

[Kimblebee]

/gemini review

[gemini-code-assist]

Code Review

This pull request does a great job of centralizing the filepath generation logic by introducing the FilePathGenerator interface. This is a solid architectural improvement that increases modularity and makes the system more extensible, as described in the PR description. The dependency injection setup is clean, and the removal of the scattered logic from various components simplifies them significantly.

I have one suggestion regarding a DRY principle violation between JcaFilePathGenerator and FakeFilePathGenerator to further improve the code structure.