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 1 commit
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
35 changes: 28 additions & 7 deletions Squirrel/SQRLUpdater.m
Original file line number Diff line number Diff line change
Expand Up @@ -304,16 +304,27 @@ - (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 _) {
NSError *error = nil;
if (![NSFileManager.defaultManager removeItemAtURL:downloadDirectory error:&error]) {
NSLog(@"Error removing temporary download directory at %@: %@", downloadDirectory, error.sqrl_verboseDescription);
}
cleanUp();
}];
}]
setNameWithFormat:@"%@ -downloadAndPrepareUpdate: %@", self, update];
Expand All @@ -339,7 +350,7 @@ - (RACSignal *)downloadBundleForUpdate:(SQRLUpdate *)update intoDirectory:(NSURL
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 @@ -359,6 +370,8 @@ - (RACSignal *)downloadBundleForUpdate:(SQRLUpdate *)update intoDirectory:(NSURL
}]
flatten]
flattenMap:^(NSData *data) {
if (data == nil) return [RACSignal return:nil];

NSURL *zipOutputURL = [downloadDirectory URLByAppendingPathComponent:zipDownloadURL.lastPathComponent];

NSError *error = nil;
Expand All @@ -370,19 +383,27 @@ - (RACSignal *)downloadBundleForUpdate:(SQRLUpdate *)update intoDirectory:(NSURL
}];
}]
doNext:^(NSURL *zipOutputURL) {
if (zipOutputURL == nil) return;

NSLog(@"Download completed to: %@", zipOutputURL);
}]
flattenMap:^(NSURL *zipOutputURL) {
return [[SQRLZipArchiver
if (zipOutputURL == nil) return [RACSignal return:nil];

return [[[[SQRLZipArchiver
unzipArchiveAtURL:zipOutputURL intoDirectoryAtURL:downloadDirectory]
ignoreValues]
concat:[RACSignal return:zipOutputURL]]
doCompleted:^{
NSError *error = nil;
if (![NSFileManager.defaultManager removeItemAtURL:zipOutputURL error:&error]) {
NSLog(@"Error removing downloaded archive at %@: %@", zipOutputURL, error.sqrl_verboseDescription);
}
}];
}]
then:^{
flattenMap:^(NSURL *zipOutputURL) {
if (zipOutputURL == nil) return [RACSignal return:nil];

return [self updateBundleMatchingCurrentApplicationInDirectory:downloadDirectory];
}]
setNameWithFormat:@"%@ -downloadBundleForUpdate: %@ intoDirectory: %@", self, update, downloadDirectory];
Expand Down