Reputation: 372
i begin to developp an iphone application and i need some advice about the NSURLSession and about how to manage properly the download and parsing of my data.
i just finish to correct a bug for the download of my data in a nsurlsession, but regarding how difficult i find to understand these asynchronus request, i think my solution is not really good... also the download bug appear with 2 different solution of download, wich make me think that i forgot to do something...
In, my project i download different xml files (and some zip files with pictures sometime) that i need to parse before to display their informations. these informations can change quickly so i want to download them again if i load my page again. i was looking for an easy way to manage all the downloads in the same way, like that i wouldn't have to re write a lot of code.
i found this project first.
With that , i just had to use that code to manage the downloads :
NSString *downloadUrl = @"https://www.url.com";
NSURL *location = [NSURL URLWithString:downloadUrl];
// DownloadManager is my version of the CTSessionOperation of the github project
DownloadManager *operation = [DownloadManager new];
operation.downloadUrl = downloadUrl;
operation.completionAction = ^(NSURL *xmlUrl, BOOL success){
dispatch_async(dispatch_get_main_queue(), ^{
if (success){
regions = [[TeamChoiceManager sharedManager] parseRegions:[NSData dataWithContentsOfURL:location]];
[self.tableView performSelectorOnMainThread:@selector(reloadData) withObject:Nil waitUntilDone:YES];
}
});
};
operation.isBackground = YES;
[operation enqueueOperation];
this code works perfectly the first time i download. but if i try to launch again the download, the download is not lauched (so no error, just, this code download once and that's all).
i correct this bug by modifying the metod (NSURLSession *)session
in CTSessionOperation
/ DownloadManager
. i put in comment the "dispatch_once" to make it works, but i don't think that it's the good solution...
i tried an other solution wich led to the same bug. i manage the download with this code :
NSString *regionsUrl= @"url";
NSURLSessionConfiguration *sessionConfig =
[NSURLSessionConfiguration defaultSessionConfiguration];
// My solution to the bug
/*NSURLSessionConfiguration *backgroundConfiguration = [NSURLSessionConfiguration
backgroundSessionConfiguration:[NSString stringWithFormat:@"com.captech.mysupersession.BackgroundSession%d",numBGSession]]; */
// numBGSession++; this is a static NSInteger
NSURLSession *session =
[NSURLSession sessionWithConfiguration:backgroundConfiguration
delegate:teamChoiceDetailViewController
delegateQueue:nil];
NSURLSessionDownloadTask *sessDLTask =
[session downloadTaskWithURL:[NSURL URLWithString:regionsUrl]];
[sessDLTask resume];
and in the delegate :
-(void)URLSession:(NSURLSession *)session
downloadTask:(NSURLSessionDownloadTask *)downloadTask
didFinishDownloadingToURL:(NSURL *)location
{
dispatch_async(dispatch_get_main_queue(), ^{
self.regions = [[TeamChoiceManager sharedManager] parseRegions:[NSData dataWithContentsOfURL:location]];
[self.tableView performSelectorOnMainThread:@selector(reloadData) withObject:Nil waitUntilDone:YES];
});
}
with this solution, i avoid the bug by creating a custom NSURLSessionConfiguration
every time i try to download.
so here we go. i'm quite confuse with this 2 solutions. i don't know if they are proper way to manage the download, i don't think i correct the bug correctly, and i must have missed something with the logic of NSURLSession
.
do you have any advice to improve these solutions or do you see one wich is much better than the other?
Upvotes: 1
Views: 1238
Reputation: 372
EDIT : if someone is looking for a more generic solution to manage networks transactions easily and properly, you can check AFNetworking which do some magic (plus you can find a lot of tutorials).
i gave up on developping something wich work for every case (especially if it's a background session because i don't need it). in the end i just created a class with a static session, wich manage the delegates for the download, and a block that i use in didFinishDownloadingToURL to manage the downloaded data. certainly not perfect but good enough for the moment.
typedef void (^CTCompletionBlock)(NSURL *location, NSError* err);
@interface DownloadManager : NSObject <NSURLSessionDelegate, NSURLSessionTaskDelegate, NSURLSessionDownloadDelegate>
@property (nonatomic, retain) NSURLSessionDownloadTask *dlTask;
@property (nonatomic, retain) NSString *location;
@property (strong) CTCompletionBlock afterDLBlock;
+ (DownloadManager *)sharedManager;
-(void)downloadTask;
@end
//
// DownloadManager.m
// MVCTest
//
// Created by
//
#import "DownloadManager.h"
#import "AppDelegate.h"
static DownloadManager *instance = nil;
static NSURLSession *session = nil;
@implementation DownloadManager
+ (DownloadManager *)sharedManager {
if (instance == nil) {
//session = [DownloadManager sessionWithConfiguration:[NSURLSessionConfiguration defaultSessionConfiguration] delegate:self delegateQueue:nil];
instance = [DownloadManager new];
}
return instance;
}
+ (id)new
{
return [[self alloc] init];
}
- (id)init{
self = [super init];
session = [NSURLSession sessionWithConfiguration:[NSURLSessionConfiguration defaultSessionConfiguration]
return self;
}
-(void)downloadTask{
self.dlTask = [session downloadTaskWithURL:[NSURL URLWithString:self.location]];
[self.dlTask resume];
}
#pragma mark - NSURLSessionDownloadDelegate
- (void)URLSession:(NSURLSession *)session downloadTask:(NSURLSessionDownloadTask *)downloadTask didFinishDownloadingToURL:(NSURL *)location {
NSError *error;
if (self.afterDLBlock){
self.afterDLBlock(location, error);
}
}
//i still have to manage the delegate...
- (void)URLSession:(NSURLSession *)session downloadTask:(NSURLSessionDownloadTask *)downloadTask didWriteData:(int64_t)bytesWritten totalBytesWritten:(int64_t)totalBytesWritten totalBytesExpectedToWrite:(int64_t)totalBytesExpectedToWrite {}
- (void)URLSession:(NSURLSession *)session downloadTask:(NSURLSessionDownloadTask *)downloadTask didResumeAtOffset:(int64_t)fileOffset expectedTotalBytes:(int64_t)expectedTotalBytes {}
#pragma mark - NSURLSessionTaskDelegate
- (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task didCompleteWithError:(NSError *)error {}
-(void)URLSession:(NSURLSession *)session didBecomeInvalidWithError:(NSError *)error{}
#pragma mark - NSURLSessionDelegate
- (void)URLSessionDidFinishEventsForBackgroundURLSession:(NSURLSession *)session {}
@end
with this class, i just need to write this code to manage the data downloaded :
typeof(self) __weak weakSelf = self;
NSString *downloadUrl = @"http://www.whatyouwant.com";
[DownloadManager sharedManager].location = downloadUrl;
[DownloadManager sharedManager].afterDLBlock = ^(NSURL *location, NSError *error) {
weakSelf.regions = [[TeamChoiceManager sharedManager] parseRegions:[NSData dataWithContentsOfURL:location]];
dispatch_sync(dispatch_get_main_queue(), ^{
[weakSelf.activityViewIndicator stopAnimating];
[weakSelf.tableView reloadData];
});
};
[[DownloadManager sharedManager] downloadTask];
i still have to manage the errors and the delegate but with taht solution i will not have a lot of code to write to download some data
Upvotes: 1
Reputation: 438467
A couple of observations:
If you use background NSURLSession
, be aware that this can be slower than a standard NSURLSession
. And, if you're doing a lot of downloads, your session could be backlogged with some of the previous requests, so when you come back, it would appear to not do anything (when in fact, it's likely just busy trying to complete the prior task requests which are trying to update a table view instance that doesn't exist or isn't visible any more). This could explain why creating a new session appears to work properly (because the new session wouldn't queue new tasks behind previously enqueued tasks, but rather run them in parallel).
I would suggest:
restoring the dispatch_once
logic (because your intuitions here are right; you definitely don't want to be making a bunch of sessions, leaving the old ones out there running); and
when you return to this view controller, cancel any pending requests before initiating new ones.
Your initial attempt, using completionAction
, is problematic because:
You are retaining the view controller. So instead of:
operation.completionAction = ^(NSURL *xmlUrl, BOOL success){
dispatch_async(dispatch_get_main_queue(), ^{
if (success){
regions = [[TeamChoiceManager sharedManager] parseRegions:[NSData dataWithContentsOfURL:location]];
[self.tableView performSelectorOnMainThread:@selector(reloadData) withObject:Nil waitUntilDone:YES];
}
});
};
You might want:
typeof(self) __weak weakSelf = self;
operation.completionAction = ^(NSURL *xmlUrl, BOOL success){
dispatch_async(dispatch_get_main_queue(), ^{
if (success){
regions = [[TeamChoiceManager sharedManager] parseRegions:[NSData dataWithContentsOfURL:location]];
[weakSelf.tableView reloadData]; // don't need that `performSelectorOnMainThread` call
}
});
};
You're also trying to use this completionAction
in conjunction with a background NSURLSession
. Note that if the app is terminated, while the download will finish in the background, this completion block will be lost (defeating the purpose in doing a background session).
If you really need to use background sessions, you should move any logic inside this completionAction
into the download delegate method itself. You cannot use this completionAction
in conjunction with background sessions and expect it to survive the termination of the app (even though the NSURLSessionDownloadTask
objects would).
Below is my original answer, in which I inveighed against the CTSessionOperation
class, because (a) there is a cognitive dissonance in using completion blocks in conjunction with background NSURLSession
(since those blocks will be lost if the app is terminated, even though the downloads will continue); and (b) it's using a non-concurrent NSOperation
for an asynchronous task, defeating many of the key benefits of using NSOperation
-based implementation. While both of these points are certainly problematic, I think these concerns are secondary to my points above, but I'll keep this here for reference.
Original answer:
What do you mean by "works perfectly the first time i download; but [not] if I try to launch again"? Are you talking about the app is terminated and downloads still happening in the background? The original CTSessionOperation
would not appear to handle that correctly, because it's attempting to use the standard trick of NSOperation
and completion blocks, but all of that is incompatible with NSURLSession
background sessions that proceed even after the app is terminated. Once an app is terminated, the NSURLSession
background requests continue to proceed, but all of the operations and completion blocks will be totally lost. I think this CTSessionOperation
has missed the mark here. You really cannot enjoy the richness of background sessions that progress even after an app is terminated and expect to preserve the operations and completion blocks you created when you first launched the background download tasks. You have to stick with NSURLSessionDownloadTask
objects and use the delegate methods only; no completion blocks or operations.
Despite my criticism of what appear to be structural flaws of CTSessionOperation
, the attempted remedy of commenting out the dispatch_once
and creating unique background sessions or creating standard foreground sessions is definitely not the right approach if you need background downloads. If anything, if you're trying to remedy the problems that CTSessionOperation
doesn't handle background sessions after the app is terminated, this is actually going in the wrong direction, methinks.
So, the question is whether you really want to enjoy the full functionality of background NSURLSession
(in which case you have to abandon the completion blocks and operation pattern of CTSessionOperation
), or whether you are ok with a non-background NSURLSession
where the downloads being terminated when the app, itself, terminates, and you only want to be able to restart them from scratch when the user fires up the app again.
Upvotes: 0