feat: Delete old downloaded videos data on device to optimize storage #452
Conversation
| @@ -124,6 +124,8 @@ public protocol DownloadManagerProtocol { | |||
| func resumeDownloading() throws | |||
| func fileUrl(for blockId: String) -> URL? | |||
| func isLargeVideosSize(blocks: [CourseBlock]) -> Bool | |||
|
|
|||
| func removeAppSupportDirectoryDeprecatedContent() | |||
There was a problem hiding this comment.
How about renaming it to removeAppSupportDirectoryUnusedContent thoughts?
| @@ -470,6 +472,60 @@ public class DownloadManager: DownloadManagerProtocol { | |||
| debugLog("SaveFile Error", error.localizedDescription) | |||
| } | |||
| } | |||
|
|
|||
| public func removeAppSupportDirectoryDeprecatedContent() { | |||
| deleteMD5HashedFolders() | |||
There was a problem hiding this comment.
How about doing the work directly here that you are doing in deleteMD5HashedFolders function?
There was a problem hiding this comment.
For our specific scenario, the suggested name removeAppSupportDirectoryUnusedContent seems fitting. However, the name deleteMD5HashedFolders explicitly describes the deletion of MD5 hashed folders (not any other content). If we integrate the implementation of deleteMD5HashedFolders() directly into removeAppSupportDirectoryUnusedContent(), it might affect the clarity of the abstraction level. Do you have any suggestions or thoughts on finding a more suitable name?
| ) | ||
| return appSupportDirectory | ||
| } catch { | ||
| print("Error getting Application Support Directory: \(error)") |
There was a problem hiding this comment.
Please use debugPrint here and in other places where applicable in code that you wrote.
OpenEdX/RouteController.swift
Outdated
| private func resetAppSupportDirectoryUserData() { | ||
| if var upgradationValue = Container.shared.resolve(CoreStorage.self) { | ||
| if upgradationValue.resetAppSupportDirectoryUserData == false { | ||
| Task { | ||
| Container.shared.resolve(DownloadManagerProtocol.self)?.removeAppSupportDirectoryDeprecatedContent() | ||
| upgradationValue.resetAppSupportDirectoryUserData = true | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
How about updating this method to guard like below thoughts?
private func resetAppSupportDirectoryUserData() {
guard var upgradationValue = Container.shared.resolve(CoreStorage.self),
let downloadManager = Container.shared.resolve(DownloadManagerProtocol.self),
upgradationValue.resetAppSupportDirectoryUserData == false
else {return}
Task {
downloadManager.removeAppSupportDirectoryDeprecatedContent()
upgradationValue.resetAppSupportDirectoryUserData = true
}
}
| } | ||
|
|
||
| private func isMD5Hash(_ folderName: String) -> Bool { | ||
| let md5Regex = "^[a-fA-F0-9]{32}$" |
There was a problem hiding this comment.
Can md5 contain uppercase letters?
There was a problem hiding this comment.
True, an MD5 hash will not contain uppercase letters. However, to be safe, if there is any customization that introduces uppercase letters, it will also delete that folder.
|
@saeedbashir @rnr Addressed suggestions. Ready for another round of review 🎉 |
|
@volodymyr-chekyrta Awaiting your review. Thanks |
LEARNER-9966: Delete old downloaded videos data on device to optimize storage
We decided not to migrate downloaded content but have to look into deleting the old downloaded videos so the app does not create useless dummy data in the learners' devices.
Technical Details:
The old application saves users' downloaded videos in the
Application Supportdirectory within folders named users' MD5 hashes. When multiple users log into the same device, separate MD5 hashed folders are created for each user, each containing that user's downloaded data.After upgrading the application, we can use the MD5 hash algorithm to delete only the data of the currently logged-in user, while the data for other users will remain intact. Because MD5 hash folder name will be generated for current logged in username. To prevent this scenario, we need to create a regex that identifies and deletes all MD5 hashed folders upon launching the upgraded application.
We can consider removing this code once we are confident that most or all users have transitioned to the new application.
Before.Update.mov
After.Update.mov