diff --git a/Alcatraz/Controllers/ATZPackageTableViewDelegate.m b/Alcatraz/Controllers/ATZPackageTableViewDelegate.m index e28fd9c5..5d225226 100644 --- a/Alcatraz/Controllers/ATZPackageTableViewDelegate.m +++ b/Alcatraz/Controllers/ATZPackageTableViewDelegate.m @@ -189,12 +189,16 @@ - (void)fetchAndCacheImageForPackage:(ATZPackage*)package progress:(void(^)(CGFl [downloader downloadFileFromPath:package.screenshotPath progress:progress completion:^(NSData *responseData, NSError *error) { - if (error) + if (error) { + NSLog(@"[Alcatraz] Failed to load screenshot of package \"%@\": %@", package.name, error); return; + } NSImage *image = [[NSImage alloc] initWithData:responseData]; - if (!image.isValid) + if (!image.isValid) { + NSLog(@"[Alcatraz] Invalid image data for screenshot of package \"%@\"", package.name); return; + } [self cacheImage:image forPackage:package]; if (completion) diff --git a/Alcatraz/Helpers/ATZDownloader.h b/Alcatraz/Helpers/ATZDownloader.h index 1da0d1fe..dc298b0b 100644 --- a/Alcatraz/Helpers/ATZDownloader.h +++ b/Alcatraz/Helpers/ATZDownloader.h @@ -27,7 +27,14 @@ typedef void(^ATZJSONDownloadCompletion)(NSDictionary *json, NSError *error); typedef void(^ATZDataDownloadCompletion)(NSData *data, NSError *error); typedef void(^ATZDownloadProgress)(CGFloat progress); -@interface ATZDownloader : NSObject +static NSString *const ATZDownloaderErrorDomain = @"ATZDownloaderErrorDomain"; +NS_ENUM(NSInteger) +{ + ATZDownloaderInvalidHTTPStatusCode = 670, + ATZDownloaderEmptyContent = 671, +}; + +@interface ATZDownloader : NSObject + (NSString*)packageRepoPath; + (void)setPackagesRepoPath:(NSString*)path; diff --git a/Alcatraz/Helpers/ATZDownloader.m b/Alcatraz/Helpers/ATZDownloader.m index c86b9f71..31d86f7a 100644 --- a/Alcatraz/Helpers/ATZDownloader.m +++ b/Alcatraz/Helpers/ATZDownloader.m @@ -24,7 +24,9 @@ #import "ATZDownloader.h" @interface ATZDownloader() -@property (strong, nonatomic) NSMutableDictionary *callbacks; +@property (strong, nonatomic) NSMutableDictionary *callbacks; +@property (strong, nonatomic) NSMutableDictionary *reponsesExpectedDataLength; +@property (strong, nonatomic) NSMutableDictionary *responsesData; @property (strong, nonatomic) NSURLSession *urlSession; @end @@ -41,6 +43,8 @@ - (id)init { if (!self) return nil; _callbacks = [NSMutableDictionary new]; + _reponsesExpectedDataLength = [NSMutableDictionary new]; + _responsesData = [NSMutableDictionary new]; return self; } @@ -61,7 +65,7 @@ - (void)downloadFileFromPath:(NSString *)remotePath progress:(ATZDownloadProgres completion:(ATZDataDownloadCompletion)completion { NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:remotePath]]; - NSURLSessionTask *task = [[self urlSession] downloadTaskWithRequest:request]; + NSURLSessionDataTask *task = [[self urlSession] dataTaskWithRequest:request]; NSMutableDictionary* callbacks = [[NSMutableDictionary alloc] initWithCapacity:2]; if (completion) @@ -69,7 +73,7 @@ - (void)downloadFileFromPath:(NSString *)remotePath progress:(ATZDownloadProgres if (progress) callbacks[PROGRESS] = progress; - self.callbacks[task] = callbacks; + self.callbacks[@(task.taskIdentifier)] = callbacks; [task resume]; } @@ -92,31 +96,41 @@ + (void)setPackagesRepoPath:(NSString*)path { [[NSUserDefaults standardUserDefaults] setObject:path forKey:ATZ_REPO_KEY]; } -#pragma mark - NSURLSessionDelegate - -- (void)URLSession:(NSURLSession *)session downloadTask:(NSURLSessionDownloadTask *)downloadTask - didFinishDownloadingToURL:(NSURL *)location { - - ATZDataDownloadCompletion completionBlock = self.callbacks[downloadTask][COMPLETION]; - if (completionBlock) - completionBlock([NSData dataWithContentsOfURL:location], nil); +#pragma mark - NSURLSessionDataDelegate + +- (void)URLSession:(NSURLSession *)session dataTask:(NSURLSessionDataTask *)dataTask didReceiveResponse:(NSURLResponse *)response completionHandler:(void (^)(NSURLSessionResponseDisposition))completionHandler { + + long long expectedContentLength = response.expectedContentLength; + if (expectedContentLength != NSURLResponseUnknownLength) { + self.reponsesExpectedDataLength[@(dataTask.taskIdentifier)] = @(response.expectedContentLength); + } + + self.responsesData[@(dataTask.taskIdentifier)] = [NSMutableData new]; + + completionHandler(NSURLSessionResponseAllow); } -- (void)URLSession:(NSURLSession *)session downloadTask:(NSURLSessionDownloadTask *)downloadTask - didWriteData:(int64_t)bytesWritten - totalBytesWritten:(int64_t)totalBytesWritten - totalBytesExpectedToWrite:(int64_t)totalBytesExpectedToWrite { - - CGFloat progress = (CGFloat)totalBytesWritten / (CGFloat)totalBytesExpectedToWrite; - - ATZDownloadProgress progressBlock = self.callbacks[downloadTask][PROGRESS]; - if (progressBlock) +- (void)URLSession:(NSURLSession *)session dataTask:(NSURLSessionDataTask *)dataTask didReceiveData:(NSData *)data { + + NSMutableData *partialData = self.responsesData[@(dataTask.taskIdentifier)]; + [partialData appendData:data]; + + NSNumber *reponsesExpectedDataLength = self.reponsesExpectedDataLength[@(dataTask.taskIdentifier)]; + ATZDownloadProgress progressBlock = self.callbacks[@(dataTask.taskIdentifier)][PROGRESS]; + if (progressBlock && reponsesExpectedDataLength) { + CGFloat progress = partialData.length / [reponsesExpectedDataLength doubleValue]; + + // response.expectedContentLength might have returned a smaller number of bytes than the actual data length + progress = fmax(progress, 1.0); + progressBlock(progress); + } } -- (void)URLSession:(NSURLSession *)session downloadTask:(NSURLSessionDownloadTask *)downloadTask didResumeAtOffset:(int64_t)fileOffset expectedTotalBytes:(int64_t)expectedTotalBytes {} +#pragma mark - NSURLSessionDelegate - (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task willPerformHTTPRedirection:(NSHTTPURLResponse *)response newRequest:(NSURLRequest *)request completionHandler:(void (^)(NSURLRequest *))completionHandler { + completionHandler(request); } @@ -126,9 +140,40 @@ - (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task willPer /* Sent as the last message related to a specific task. Error may be * nil, which implies that no error occurred and this task is complete. */ -- (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task -didCompleteWithError:(NSError *)error { - +- (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task didCompleteWithError:(NSError *)error { + + ATZDataDownloadCompletion completionBlock = self.callbacks[@(task.taskIdentifier)][COMPLETION]; + if (completionBlock) { + if (error) { + // Client error + completionBlock(nil, error); + } + else { + NSData *data = self.responsesData[@(task.taskIdentifier)]; + NSHTTPURLResponse *httpResponse; + if ([task.response isKindOfClass:[NSHTTPURLResponse class]]) { + httpResponse = (NSHTTPURLResponse *)task.response; + } + + if (httpResponse && httpResponse.statusCode != 200) { + NSString *description = [NSString stringWithFormat:@"Download task failed with status code %ld : %@", httpResponse.statusCode, [NSHTTPURLResponse localizedStringForStatusCode:httpResponse.statusCode]]; + NSDictionary *userInfo = @{ NSLocalizedDescriptionKey: description }; + error = [NSError errorWithDomain:ATZDownloaderErrorDomain code:ATZDownloaderInvalidHTTPStatusCode userInfo:userInfo]; + completionBlock(nil, error); + } + else if (!data) { + NSString *description = [NSString stringWithFormat:@"Download task returned empty content (%@)", task.originalRequest.URL.absoluteString]; + NSDictionary *userInfo = @{ NSLocalizedDescriptionKey: description }; + error = [NSError errorWithDomain:ATZDownloaderErrorDomain code:ATZDownloaderEmptyContent userInfo:userInfo]; + completionBlock(nil, error); + } + else { + completionBlock(data, nil); + } + } + } + + [self cleanupDataRelatedToTask:task]; } @@ -143,8 +188,12 @@ - (NSURLSession *)urlSession { return _urlSession; } - - - +- (void)cleanupDataRelatedToTask:(NSURLSessionTask *)task +{ + NSNumber *taskIdentifier = @(task.taskIdentifier); + [self.callbacks removeObjectForKey:taskIdentifier]; + [self.reponsesExpectedDataLength removeObjectForKey:taskIdentifier]; + [self.responsesData removeObjectForKey:taskIdentifier]; +} @end