refactor: make diskmanager package respect locked clips during cleanup #455
Conversation
- Introduce database interface to retrieve locked note clip paths - Modify audio file cleanup to skip locked clips - Add locked file tracking in file information structure - Extend cleanup methods to respect locked clip protection - Update file scanning to check for locked clips during retention policy processing
- Extend mockStore to include GetLockedNotesClipPaths method - Prepare test infrastructure for locked notes clip path retrieval
WalkthroughThe changes integrate a datastore interface for enhanced audio clip cleanup and locked file management. Function signatures across the analysis and disk management modules have been updated to pass this datastore parameter. New methods are introduced to retrieve locked note clip paths, and additional logic is implemented to prevent deletion of locked files. These modifications standardize the handling of audio files and ensure better coordination between file processing and database interactions. Changes
Sequence Diagram(s)sequenceDiagram
participant RT as RealTimeAnalysis
participant SC as startClipCleanupMonitor
participant CCM as clipCleanupMonitor
participant DM as DiskManager
participant FU as FileUtils
participant DB as datastore Interface
RT->>SC: Invoke with datastore parameter
SC->>CCM: Start clip cleanup monitor (passes datastore)
CCM->>DM: Call AgeBased/UsageBasedCleanup with datastore
DM->>FU: Request GetAudioFiles(baseDir, allowedFileTypes, db, debug)
FU->>DB: Fetch locked clip paths via getLockedClips()
DB-->>FU: Return list of locked clips
FU->>DM: Provide FileInfo list with Locked status
Possibly related PRs
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
internal/diskmanager/file_utils.go (3)
17-18: Consider making file extension settings configurable.
Currently,allowedFileTypesis hardcoded, which might be sufficient for your immediate needs. However, externalizing or centralizing this list in configuration can improve flexibility for future changes.
30-33: Consider renaming the interface for clarity.
Naming the interface simply asInterfacemight lead to confusion and reduce clarity. Give it a more descriptive name (e.g.,DatastoreInterface,LockedClipProvider, etc.) to clearly convey its purpose.
69-69: Evaluate parameter ordering in the function signature.
Passingdbin the middle can be slightly confusing. Consider grouping parameters into a struct or reordering them to improve readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
internal/analysis/realtime.go(4 hunks)internal/datastore/interfaces.go(2 hunks)internal/diskmanager/file_utils.go(4 hunks)internal/diskmanager/policy_age.go(3 hunks)internal/diskmanager/policy_usage.go(3 hunks)internal/imageprovider/imageprovider_test.go(1 hunks)
🔇 Additional comments (17)
internal/diskmanager/file_utils.go (6)
27-28: Field addition looks good.
Introducing theLockedfield is an effective way to mark protected files and prevent their deletion.
72-77: Fetching locked clips is correctly handled.
Graceful handling of potential database errors helps maintain code robustness.
78-80: Debug logs offer helpful insights.
Including the count of locked clips can be invaluable for auditing and troubleshooting.
93-94: Verify use of filename-only comparisons.
Currently,isLockedClipsetsLockedby matching just the file’s base name against locked paths. If two different directories contain files with identical names, this logic might incorrectly lock or unlock them. Confirm that ignoring directory structure is correct for your workflow.
175-181: Helper function is concise and clear.
ThegetLockedClipsfunction gracefully handles a nil interface check and delegates retrieval to the datastore.
183-193: Double-check directory-level distinction for locked clips.
As mentioned, this function only compares the base filenames against locked ones. Ensure you won’t encounter collisions if identical filenames exist in different directories.internal/diskmanager/policy_age.go (3)
15-15: Update to accept a datastore interface is appropriate.
Allowing the function to query locked clips is foundational to ensuring that they aren’t deleted prematurely.
33-34: Using the shared list of allowed file types is consistent.
Switching toallowedFileTypeskeeps the logic centralized in a single location.
60-66: Correctly skipping locked files.
This approach neatly prevents protected clips from being removed.internal/diskmanager/policy_usage.go (3)
21-21: Enhanced function signature is well-aligned.
AllowingUsageBasedCleanupto leverage a database parameter ensures locked clips are respected.
49-50: Loading filtered files viaallowedFileTypes.
Delegating the file type list tofile_utils.gopromotes consistency across cleanup policies.
86-92: Skipping locked files is effectively integrated.
Adhering to the newLockedfield ensures these clips remain untouched during usage-based cleanup.internal/analysis/realtime.go (2)
157-157: LGTM! Passing datastore to cleanup monitor.The change correctly passes the datastore instance to the cleanup monitor, enabling it to check for locked clips during cleanup.
281-281: LGTM! Passing datastore to cleanup functions.The changes correctly pass the datastore instance to both cleanup functions, enabling them to respect locked clips during cleanup.
Also applies to: 288-288
internal/imageprovider/imageprovider_test.go (1)
169-169: LGTM! Mock implementation of GetLockedNotesClipPaths.The mock implementation correctly implements the required interface method with a minimal implementation suitable for testing.
internal/datastore/interfaces.go (2)
58-58: LGTM! New interface method for retrieving locked clip paths.The interface is correctly extended with the new method to support the locked clips feature.
879-895: LGTM! Well-structured implementation of GetLockedNotesClipPaths.The implementation efficiently retrieves locked clip paths:
- Uses JOIN to get locked notes in a single query
- Filters out notes without clips
- Returns only the clip paths using Pluck
- Includes proper error handling
Audio clips of locked detections are not removed during disk cleanup process