[MC-1563] File downloading #339
Merged
Conversation
This file contains 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
Adds callback for each file and all files download
Updated unit tests.
…, file variables. - Added expiry logic for files downloaded. - Added unit tests.
… to the url to download the file. - Added methods for image preloading cases. - Added unit test cases.
- Added changes for CS InApps to use the new CTFileDownloader class for image preloading. - Removed previous usage of CTInAppImagePrefetchManager class for image preloading.
nzagorchev
requested changes
Jun 21, 2024
There was a problem hiding this comment.
- Use custom timeout for the download requests. NSURLSession shared session uses default timeouts which are too long.
- Handle status code of the download request. Accept only 200 OK. In case of 404 or 500 status code, the error is nil and the code will continue to write the data from the response.
- Completion handler is set as nullable but there is no nil check.
- Completion handler is not called at all if URLs are empty array.
- UserDefaults data is not migrated to new keys and removed.
- Get methods should follow convention and not have get in name.
- Unit tests are not using the completion blocks but arbitrary dispatch time thus taking too much time to complete.
The active and inactive assets logic is correct and provides information where an asset is used from. However, this brings too much complexity for our use case. It is better to now use expiry timestamp for each file URL and update this accordingly.
CleverTapSDK/CTConstants.h
Outdated
| @@ -140,6 +140,10 @@ extern NSString *CLTAP_PROFILE_IDENTITY_KEY; | |||
| #define CLTAP_PREFS_CS_INAPP_INACTIVE_ASSETS @"cs_inapp_inactive_assets" | |||
There was a problem hiding this comment.
Those are no longer used
CleverTapSDK/CTFileDownloadManager.m
Outdated
|
|
||
| -(void)downloadSingleFile:(NSURL *)url | ||
| completed:(void(^)(BOOL success))completedBlock { | ||
| NSURLSession *session = [NSURLSession sharedSession]; |
There was a problem hiding this comment.
Configure the session timeout.
CleverTapSDK/CTFileDownloadManager.m
Outdated
| NSURLSession *session = [NSURLSession sharedSession]; | ||
| NSURLSessionDownloadTask *downloadTask = [session downloadTaskWithURL:url | ||
| completionHandler:^(NSURL *location, NSURLResponse *response, NSError *error) { | ||
| if (error) { |
There was a problem hiding this comment.
Check the response code to be 200 OK, otherwise call the completion and return.
CleverTapSDK/CTFileDownloadManager.m
Outdated
|
|
||
| // Save the file to a desired location | ||
| NSString *filePath = [self.documentsDirectory stringByAppendingPathComponent:[response suggestedFilename]]; | ||
| [fileData writeToFile:filePath atomically:YES]; |
There was a problem hiding this comment.
It is more efficient to use [fileManager moveItemAtURL:location toURL:destinationURL error:&fileError];
CleverTapSDK/CTFileDownloadManager.m
Outdated
|
|
||
| - (void)deleteSingleFile:(NSURL *)url | ||
| completed:(void(^)(BOOL success))completedBlock { | ||
| NSString *filePath = [self.documentsDirectory stringByAppendingPathComponent:[url lastPathComponent]]; |
There was a problem hiding this comment.
Validate the url. Otherwise, if there is an edge case and it is invalid, it will delete the documents folder.
| // NSLog(@"All files downloaded with status: %@", status); | ||
| }]; | ||
|
|
||
| dispatch_time_t delay = dispatch_time(DISPATCH_TIME_NOW, 2.0 * NSEC_PER_SEC); |
There was a problem hiding this comment.
The completionBlock can be used instead of unnecessary wait.
|
|
||
| #pragma mark Private methods | ||
|
|
||
| - (void)addAllStubs { |
| [CTPreferences putInt:ts forKey:[self storageKeyWithSuffix:CLTAP_FILE_ASSETS_LAST_DELETED_TS]]; | ||
| } | ||
|
|
||
| - (NSArray<NSString *> *)generateFileURLs:(int)count { |
There was a problem hiding this comment.
Throws an exception if count > 3
| - (NSArray<NSURL *> *)generateFileURLs:(int)count { | ||
| NSMutableArray *arr = [NSMutableArray new]; | ||
| for (int i = 0; i < count; i++) { | ||
| NSString *urlString = [[NSString alloc] initWithFormat:@"https://clevertap.com/%@.%@",fileURLMatch, fileURLTypes[i]]; |
There was a problem hiding this comment.
Throws exception if count >= 3
| #pragma mark Private methods | ||
|
|
||
| - (void)addAllStubs { | ||
| [HTTPStubs stubRequestsPassingTest:^BOOL(NSURLRequest *request) { |
There was a problem hiding this comment.
No timeout and error testing of the requests.
…-sdk into file_downloading # Conflicts: # CleverTapSDK/CTConstants.h # CleverTapSDK/CTFileDownloader.m
Remove all files removes the files inside the directory Add Unit tests
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Added class
CTFileDownloadManagerandCTFileDownloaderto handle file downloading, their callback and expiry handling for all file types together.