Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't error if our conditional GET fails. #176

Merged
merged 2 commits into from
Jun 15, 2016
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 51 additions & 34 deletions Squirrel/SQRLUpdater.m
Original file line number Diff line number Diff line change
Expand Up @@ -304,26 +304,69 @@ - (RACSignal *)downloadAndPrepareUpdate:(SQRLUpdate *)update {
return [[[self
uniqueTemporaryDirectoryForUpdate]
flattenMap:^(NSURL *downloadDirectory) {
void (^cleanUp)(void) = ^{
NSError *error;
if (![NSFileManager.defaultManager removeItemAtURL:downloadDirectory error:&error]) {
NSLog(@"Error removing temporary download directory at %@: %@", downloadDirectory, error.sqrl_verboseDescription);
}
};

return [[[self
downloadBundleForUpdate:update intoDirectory:downloadDirectory]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way we could have the download signal return the already downloaded URL from the cache to save these if nil branches?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we really need is a monad transformer for RAC.

But anyways, I like that idea, but essentially all the branches are avoiding doing things with the zip, which we understandably don't keep around after unzipping.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we really need is a monad transformer for RAC.

Tell me more :thinking_face:

But anyways, I like that idea, but essentially all the branches are avoiding doing things with the zip

Hmm that’s a good point, can we structure it so that we return a signal from inside the conditional get that is flattened, then we can branch on whether we have a zip to decompress or can fulfill the unarchived update from the cache?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reorganized so we can avoid all the annoying nil checks. We're still providing a nil bundle because we want to distinguish between the case where we just downloaded the update and want to verify it, and the case where we already have it and just need to clean up the temp directory.

flattenMap:^(NSBundle *updateBundle) {
// If the bundle is nil it means our conditional GET told us
// we already downloaded the update. So just clean up.
if (updateBundle == nil) {
cleanUp();
return [RACSignal empty];
}

return [self verifyAndPrepareUpdate:update fromBundle:updateBundle];
}]
doError:^(id _) {
cleanUp();
}];
}]
setNameWithFormat:@"%@ -downloadAndPrepareUpdate: %@", self, update];
}

- (RACSignal *)unarchiveAndPrepareData:(NSData *)data withName:(NSString *)name intoDirectory:(NSURL *)downloadDirectory {
return [[[[[RACSignal
defer:^{
NSURL *zipOutputURL = [downloadDirectory URLByAppendingPathComponent:name];
NSError *error = nil;
if ([data writeToURL:zipOutputURL options:NSDataWritingAtomic error:&error]) {
return [RACSignal return:zipOutputURL];
} else {
return [RACSignal error:error];
}
}]
doNext:^(NSURL *zipOutputURL) {
NSLog(@"Download completed to: %@", zipOutputURL);
}]
flattenMap:^(NSURL *zipOutputURL) {
return [[[[SQRLZipArchiver
unzipArchiveAtURL:zipOutputURL intoDirectoryAtURL:downloadDirectory]
ignoreValues]
concat:[RACSignal return:zipOutputURL]]
doCompleted:^{
NSError *error = nil;
if (![NSFileManager.defaultManager removeItemAtURL:downloadDirectory error:&error]) {
NSLog(@"Error removing temporary download directory at %@: %@", downloadDirectory, error.sqrl_verboseDescription);
if (![NSFileManager.defaultManager removeItemAtURL:zipOutputURL error:&error]) {
NSLog(@"Error removing downloaded archive at %@: %@", zipOutputURL, error.sqrl_verboseDescription);
}
}];
}]
setNameWithFormat:@"%@ -downloadAndPrepareUpdate: %@", self, update];
flattenMap:^(NSURL *zipOutputURL) {
return [self updateBundleMatchingCurrentApplicationInDirectory:downloadDirectory];
}]
setNameWithFormat:@"%@ -unarchiveAndPrepareData:withName: %@ intoDirectory: %@", self, name, downloadDirectory];
}

- (RACSignal *)downloadBundleForUpdate:(SQRLUpdate *)update intoDirectory:(NSURL *)downloadDirectory {
NSParameterAssert(update != nil);
NSParameterAssert(downloadDirectory != nil);

return [[[[[RACSignal
return [[RACSignal
defer:^{
NSURL *zipDownloadURL = update.updateURL;
NSMutableURLRequest *zipDownloadRequest = [NSMutableURLRequest requestWithURL:zipDownloadURL];
Expand All @@ -332,14 +375,14 @@ - (RACSignal *)downloadBundleForUpdate:(SQRLUpdate *)update intoDirectory:(NSURL
[zipDownloadRequest setValue:self.etag forHTTPHeaderField:@"If-None-Match"];
}

return [[[[NSURLConnection
return [[[NSURLConnection
rac_sendAsynchronousRequest:zipDownloadRequest]
reduceEach:^(NSURLResponse *response, NSData *bodyData) {
if ([response isKindOfClass:NSHTTPURLResponse.class]) {
NSHTTPURLResponse *httpResponse = (id)response;

if (httpResponse.statusCode == 304 /* Not Modified */) {
return [RACSignal empty];
return [RACSignal return:nil];
}

if (!(httpResponse.statusCode >= 200 && httpResponse.statusCode <= 299)) {
Expand All @@ -355,35 +398,9 @@ - (RACSignal *)downloadBundleForUpdate:(SQRLUpdate *)update intoDirectory:(NSURL
self.etag = httpResponse.allHeaderFields[@"ETag"];
}

return [RACSignal return:bodyData];
return [self unarchiveAndPrepareData:bodyData withName:zipDownloadURL.lastPathComponent intoDirectory:downloadDirectory];
}]
flatten]
flattenMap:^(NSData *data) {
NSURL *zipOutputURL = [downloadDirectory URLByAppendingPathComponent:zipDownloadURL.lastPathComponent];

NSError *error = nil;
if ([data writeToURL:zipOutputURL options:NSDataWritingAtomic error:&error]) {
return [RACSignal return:zipOutputURL];
} else {
return [RACSignal error:error];
}
}];
}]
doNext:^(NSURL *zipOutputURL) {
NSLog(@"Download completed to: %@", zipOutputURL);
}]
flattenMap:^(NSURL *zipOutputURL) {
return [[SQRLZipArchiver
unzipArchiveAtURL:zipOutputURL intoDirectoryAtURL:downloadDirectory]
doCompleted:^{
NSError *error = nil;
if (![NSFileManager.defaultManager removeItemAtURL:zipOutputURL error:&error]) {
NSLog(@"Error removing downloaded archive at %@: %@", zipOutputURL, error.sqrl_verboseDescription);
}
}];
}]
then:^{
return [self updateBundleMatchingCurrentApplicationInDirectory:downloadDirectory];
flatten];
}]
setNameWithFormat:@"%@ -downloadBundleForUpdate: %@ intoDirectory: %@", self, update, downloadDirectory];
}
Expand Down