From 746018be6204da738cf79dbaa8cbaa0ac351857c Mon Sep 17 00:00:00 2001 From: relikd Date: Sun, 9 Dec 2018 19:37:45 +0100 Subject: [PATCH] Refactoring Part 4: Update Timer and Pausing --- baRSS/Categories/FeedMeta+Ext.h | 1 + baRSS/Categories/FeedMeta+Ext.m | 18 +- baRSS/Constants.h | 4 + baRSS/FeedDownload.h | 12 +- baRSS/FeedDownload.m | 247 +++++++++++--------- baRSS/Preferences/Feeds Tab/SettingsFeeds.m | 10 +- baRSS/Status Bar Menu/BarMenu.m | 16 +- 7 files changed, 180 insertions(+), 128 deletions(-) diff --git a/baRSS/Categories/FeedMeta+Ext.h b/baRSS/Categories/FeedMeta+Ext.h index 5e19e54..38abaa5 100644 --- a/baRSS/Categories/FeedMeta+Ext.h +++ b/baRSS/Categories/FeedMeta+Ext.h @@ -27,6 +27,7 @@ - (void)calculateAndSetScheduled; - (void)setEtag:(NSString*)etag modified:(NSString*)modified; +- (void)setEtagAndModified:(NSHTTPURLResponse*)http; - (BOOL)setURL:(NSString*)url refresh:(int32_t)refresh unit:(int16_t)unit; - (NSString*)readableRefreshString; diff --git a/baRSS/Categories/FeedMeta+Ext.m b/baRSS/Categories/FeedMeta+Ext.m index feb5d37..bf94eb9 100644 --- a/baRSS/Categories/FeedMeta+Ext.m +++ b/baRSS/Categories/FeedMeta+Ext.m @@ -24,12 +24,16 @@ @implementation FeedMeta (Ext) -/// Increment @c errorCount (max. 19) and set new @c scheduled (2^N seconds, max. 6 days). +/// Increment @c errorCount and set new @c scheduled date (2^N minutes, max. 5.7 days). - (void)setErrorAndPostponeSchedule { - int16_t n = self.errorCount + 1; - self.errorCount = (n < 1 ? 1 : (n > 19 ? 19 : n)); // between: 2 sec and 6 days - NSTimeInterval retryWaitTime = pow(2, self.errorCount); // 2^n seconds + if (self.errorCount < 0) + self.errorCount = 0; + int16_t n = self.errorCount + 1; // always increment errorCount (can be used to indicate bad feeds) + NSTimeInterval retryWaitTime = pow(2, (n > 13 ? 13 : n)) * 60; // 2^N (between: 2 minutes and 5.7 days) + self.errorCount = n; self.scheduled = [NSDate dateWithTimeIntervalSinceNow:retryWaitTime]; + // TODO: remove logging + NSLog(@"ERROR: Feed download failed: %@ (errorCount: %d)", self.url, n); } /// Calculate date from @c refreshNum and @c refreshUnit and set as next scheduled feed update. @@ -44,6 +48,12 @@ if (![self.modified isEqualToString:modified]) self.modified = modified; } +/// Read header field "Etag" and "Date" and set @c .etag and @c .modified. +- (void)setEtagAndModified:(NSHTTPURLResponse*)http { + NSDictionary *header = [http allHeaderFields]; + [self setEtag:header[@"Etag"] modified:header[@"Date"]]; // @"Expires", @"Last-Modified" +} + /** Set download url and refresh interval (popup button selection). @note Only values that differ will be updated. diff --git a/baRSS/Constants.h b/baRSS/Constants.h index 363a1be..b522dbb 100644 --- a/baRSS/Constants.h +++ b/baRSS/Constants.h @@ -23,6 +23,10 @@ #ifndef Constants_h #define Constants_h +// TODO: Add support for media player? +// +// TODO: Disable 'update all' menu item during update? + static NSString *kNotificationFeedUpdated = @"baRSS-notification-feed-updated"; static NSString *kNotificationNetworkStatusChanged = @"baRSS-notification-network-status-changed"; static NSString *kNotificationTotalUnreadCountChanged = @"baRSS-notification-total-unread-count-changed"; diff --git a/baRSS/FeedDownload.h b/baRSS/FeedDownload.h index c67e4e5..0125a90 100644 --- a/baRSS/FeedDownload.h +++ b/baRSS/FeedDownload.h @@ -24,9 +24,15 @@ #import @interface FeedDownload : NSObject -+ (void)newFeed:(NSString *)url block:(void(^)(RSParsedFeed *feed, NSError* error, NSHTTPURLResponse* response))block; +// Register for network change notifications + (void)registerNetworkChangeNotification; + (void)unregisterNetworkChangeNotification; -+ (BOOL)isNetworkReachable; -+ (void)scheduleNextUpdateForced:(BOOL)flag; +// Scheduled feed update ++ (void)newFeed:(NSString *)url block:(void(^)(RSParsedFeed *feed, NSError* error, NSHTTPURLResponse* response))block; ++ (void)scheduleUpdateForUpcomingFeeds; ++ (void)forceUpdateAllFeeds; +// User interaction ++ (BOOL)allowNetworkConnection; ++ (BOOL)isPaused; ++ (void)setPaused:(BOOL)flag; @end diff --git a/baRSS/FeedDownload.m b/baRSS/FeedDownload.m index 73d335f..b8b265f 100644 --- a/baRSS/FeedDownload.m +++ b/baRSS/FeedDownload.m @@ -30,10 +30,126 @@ static SCNetworkReachabilityRef _reachability = NULL; static BOOL _isReachable = NO; +static BOOL _updatePaused = NO; +static BOOL _nextUpdateIsForced = NO; @implementation FeedDownload +#pragma mark - User Interaction - + +/// @return @c YES if current network state is reachable and updates are not paused by user. ++ (BOOL)allowNetworkConnection { + return (_isReachable && !_updatePaused); +} + +/// @return @c YES if update is paused by user. ++ (BOOL)isPaused { + return _updatePaused; +} + +/// Set paused flag and cancel timer regardless of network connectivity. ++ (void)setPaused:(BOOL)flag { + _updatePaused = flag; + if (_updatePaused) + [self pauseUpdates]; + else + [self resumeUpdates]; +} + +/// Cancel current timer and stop any updates until enabled again. ++ (void)pauseUpdates { + [self scheduleTimer:nil]; +} + +/// Start normal (non forced) schedule if network is reachable. ++ (void)resumeUpdates { + if (_isReachable) + [self scheduleUpdateForUpcomingFeeds]; +} + + +#pragma mark - Update Feed Timer - + + +/** + Get date of next up feed and start the timer. + */ ++ (void)scheduleUpdateForUpcomingFeeds { + if (![self allowNetworkConnection]) // timer will restart once connection exists + return; + NSDate *nextTime = [StoreCoordinator nextScheduledUpdate]; + if (!nextTime) return; // no timer means no feeds to update + if ([nextTime timeIntervalSinceNow] < 1) { // mostly, if app was closed for a long time + nextTime = [NSDate dateWithTimeIntervalSinceNow:1]; + } + [self scheduleTimer:nextTime]; +} + +/** + Start download of all feeds (immediatelly) regardless of @c .scheduled property. + */ ++ (void)forceUpdateAllFeeds { + if (![self allowNetworkConnection]) // timer will restart once connection exists + return; + _nextUpdateIsForced = YES; + [self scheduleTimer:[NSDate dateWithTimeIntervalSinceNow:0.2]]; +} + +/** + Set new @c .fireDate and @c .tolerance for update timer. + + @param nextTime If @c nil timer will be disabled with a @c .fireDate very far in the future. + */ ++ (void)scheduleTimer:(NSDate*)nextTime { + static NSTimer *timer; + if (!timer) { + timer = [NSTimer timerWithTimeInterval:NSTimeIntervalSince1970 target:[self class] selector:@selector(updateTimerCallback) userInfo:nil repeats:YES]; + [[NSRunLoop mainRunLoop] addTimer:timer forMode:NSRunLoopCommonModes]; + } + if (!nextTime) + nextTime = [NSDate dateWithTimeIntervalSinceNow:NSTimeIntervalSince1970]; + NSTimeInterval tolerance = [nextTime timeIntervalSinceNow] * 0.15; + timer.tolerance = (tolerance < 1 ? 1 : tolerance); // at least 1 sec + timer.fireDate = nextTime; +} + +/** + Called when schedule timer runs out (earliest @c .schedule date). Or if forced by user request. + */ ++ (void)updateTimerCallback { + if (![self allowNetworkConnection]) + return; + NSLog(@"fired"); + + __block NSManagedObjectContext *childContext = [StoreCoordinator createChildContext]; + NSArray *list = [StoreCoordinator getListOfFeedsThatNeedUpdate:_nextUpdateIsForced inContext:childContext]; + _nextUpdateIsForced = NO; + if (list.count == 0) { + NSLog(@"ERROR: Something went wrong, timer fired too early."); + [childContext reset]; + childContext = nil; + // thechnically should never happen, anyway we need to reset the timer + [self resumeUpdates]; + return; // nothing to do here + } + dispatch_group_t group = dispatch_group_create(); + for (Feed *feed in list) { + [self downloadFeed:feed group:group]; + } + dispatch_group_notify(group, dispatch_get_main_queue(), ^{ + [StoreCoordinator saveContext:childContext andParent:YES]; + [[NSNotificationCenter defaultCenter] postNotificationName:kNotificationFeedUpdated object:[list valueForKeyPath:@"objectID"]]; + [childContext reset]; + childContext = nil; + [self resumeUpdates]; + }); +} + + +#pragma mark - Download RSS Feed - + + /// @return New request with no caching policy and timeout interval of 30 seconds. + (NSMutableURLRequest*)newRequestURL:(NSString*)url { NSMutableURLRequest *req = [NSMutableURLRequest requestWithURL:[NSURL URLWithString:url]]; @@ -76,72 +192,6 @@ static BOOL _isReachable = NO; }] resume]; } - -#pragma mark - Update existing feeds - - - -/** - Get date of next update schedule and start @c updateTimer. - - @param forceUpdate If @c YES all feeds will be downloaded regardless of scheduled date. - */ -+ (void)scheduleNextUpdateForced:(BOOL)forceUpdate { - static NSTimer *_updateTimer; - @synchronized (_updateTimer) { // TODO: dig into analyzer warning - if (_updateTimer) { - [_updateTimer invalidate]; - _updateTimer = nil; - } - } - if (!_isReachable) return; // cancel timer entirely (will be restarted once connection exists) - NSDate *nextTime = [NSDate dateWithTimeIntervalSinceNow:0.2]; - if (!forceUpdate) { - nextTime = [StoreCoordinator nextScheduledUpdate]; - if (!nextTime) return; // no timer means no feeds to update - if ([nextTime timeIntervalSinceNow] < 0) { // mostly, if app was closed for a long time - nextTime = [NSDate dateWithTimeIntervalSinceNow:2]; // TODO: retry in 2 sec? - } - } - NSTimeInterval tolerance = [nextTime timeIntervalSinceNow] * 0.15; - _updateTimer = [NSTimer timerWithTimeInterval:0 target:[self class] selector:@selector(scheduledUpdateTimer:) userInfo:@(forceUpdate) repeats:NO]; - _updateTimer.fireDate = nextTime; - _updateTimer.tolerance = (tolerance < 1 ? 1 : tolerance); // at least 1 sec - [[NSRunLoop mainRunLoop] addTimer:_updateTimer forMode:NSRunLoopCommonModes]; -} - -/** - Called when schedule timer has run out (earliest scheduled date). Or if forced by user request. - - @param timer @c NSTimer @c .userInfo should contain a @c BOOL value whether to force an update of all feeds @c (YES). - */ -+ (void)scheduledUpdateTimer:(NSTimer*)timer { - NSLog(@"fired"); - BOOL forceAll = [timer.userInfo boolValue]; - // TODO: check internet connection - // TODO: disable menu item 'update all' during update - __block NSManagedObjectContext *childContext = [StoreCoordinator createChildContext]; - NSArray *list = [StoreCoordinator getListOfFeedsThatNeedUpdate:forceAll inContext:childContext]; - if (list.count == 0) { - NSLog(@"ERROR: Something went wrong, timer fired too early."); - [childContext reset]; - childContext = nil; - // thechnically should never happen, anyway we need to reset the timer - [self scheduleNextUpdateForced:NO]; // NO, since forceAll will get ALL items and shouldn't be 0 - return; // nothing to do here - } - dispatch_group_t group = dispatch_group_create(); - for (Feed *feed in list) { - [self downloadFeed:feed group:group]; - } - dispatch_group_notify(group, dispatch_get_main_queue(), ^{ - [StoreCoordinator saveContext:childContext andParent:YES]; - [[NSNotificationCenter defaultCenter] postNotificationName:kNotificationFeedUpdated object:[list valueForKeyPath:@"objectID"]]; - [childContext reset]; - childContext = nil; - [self scheduleNextUpdateForced:NO]; // after forced update, continue regular cycle - }); -} - /** Start download request with existing @c Feed object. Reuses etag and modified headers. @@ -149,56 +199,41 @@ static BOOL _isReachable = NO; @param group Mutex to count completion of all downloads. */ + (void)downloadFeed:(Feed*)feed group:(dispatch_group_t)group { - if (!_isReachable) return; + if (![self allowNetworkConnection]) + return; dispatch_group_enter(group); [[[NSURLSession sharedSession] dataTaskWithRequest:[self newRequest:feed.meta] completionHandler:^(NSData * _Nullable data, NSURLResponse * _Nullable response, NSError * _Nullable error) { [feed.managedObjectContext performBlock:^{ // core data block inside of url session block; otherwise access will EXC_BAD_INSTRUCTION if (error) { [feed.meta setErrorAndPostponeSchedule]; - // TODO: remove logging - NSLog(@"Error loading: %@ (%d)", response.URL, feed.meta.errorCount); } else { - feed.meta.errorCount = 0; // reset counter - [self downloadSuccessful:data forFeed:feed response:(NSHTTPURLResponse*)response]; + [feed.meta setEtagAndModified:(NSHTTPURLResponse*)response]; + [feed.meta calculateAndSetScheduled]; + + if ([(NSHTTPURLResponse*)response statusCode] != 304) { // only parse if modified + // should be fine to call synchronous since dataTask is already in the background (always? proof?) + RSXMLData *xml = [[RSXMLData alloc] initWithData:data urlString:feed.meta.url]; + RSParsedFeed *parsed = RSParseFeedSync(xml, NULL); + if (parsed && parsed.articles.count > 0) { + [feed updateWithRSS:parsed postUnreadCountChange:YES]; + feed.meta.errorCount = 0; // reset counter + } else { + [feed.meta setErrorAndPostponeSchedule]; // replaces date of 'calculateAndSetScheduled' + } + } + // TODO: save changes for this feed only? + //[[NSNotificationCenter defaultCenter] postNotificationName:kNotificationFeedUpdated object:feed.objectID]; } dispatch_group_leave(group); }]; }] resume]; } -/** - Parse RSS feed data and save to persistent store. If HTTP 304 (not modified) skip feed evaluation. - @param data Raw data from request. - @param feed @c Feed on which the update is executed. - @param http Download response containing the statusCode and etag / modified headers. - */ -+ (void)downloadSuccessful:(NSData*)data forFeed:(Feed*)feed response:(NSHTTPURLResponse*)http { - if ([http statusCode] != 304) { - // should be fine to call synchronous since dataTask is already in the background (always? proof?) - RSXMLData *xml = [[RSXMLData alloc] initWithData:data urlString:feed.meta.url]; - RSParsedFeed *parsed = RSParseFeedSync(xml, NULL); - if (parsed) { - // TODO: add support for media player? - // - [feed updateWithRSS:parsed postUnreadCountChange:YES]; - } - } - [feed.meta setEtag:[http allHeaderFields][@"Etag"] modified:[http allHeaderFields][@"Date"]]; // @"Expires", @"Last-Modified" - // Don't update redirected url since it happened in the background; User may not recognize url - [feed.meta calculateAndSetScheduled]; - // TODO: save changes for this feed only? -// [[NSNotificationCenter defaultCenter] postNotificationName:kNotificationFeedUpdated object:feed.objectID]; -} +#pragma mark - Network Connection & Reachability - -#pragma mark - Network Connection - - - -/// External getter to check wheter current network state is reachable. -+ (BOOL)isNetworkReachable { return _isReachable; } - /// Set callback on @c self to listen for network reachability changes. + (void)registerNetworkChangeNotification { // https://stackoverflow.com/questions/11240196/notification-when-wifi-connected-os-x @@ -229,18 +264,16 @@ static BOOL _isReachable = NO; /// Called when network interface or reachability changes. static void networkReachabilityCallback(SCNetworkReachabilityRef target, SCNetworkConnectionFlags flags, void *object) { - if (_reachability == NULL) - return; + if (_reachability == NULL) return; _isReachable = [FeedDownload hasConnectivity:flags]; - [[NSNotificationCenter defaultCenter] postNotificationName:kNotificationNetworkStatusChanged - object:[NSNumber numberWithBool:_isReachable]]; - if (_isReachable) { + [[NSNotificationCenter defaultCenter] postNotificationName:kNotificationNetworkStatusChanged object:@(_isReachable)]; + if (_isReachable) { NSLog(@"reachable"); + [FeedDownload resumeUpdates]; } else { NSLog(@"not reachable"); + [FeedDownload pauseUpdates]; } - // schedule regardless of state (if not reachable timer will be canceled) - [FeedDownload scheduleNextUpdateForced:NO]; } /// @return @c YES if network connection established. diff --git a/baRSS/Preferences/Feeds Tab/SettingsFeeds.m b/baRSS/Preferences/Feeds Tab/SettingsFeeds.m index d00b70a..404a9d3 100644 --- a/baRSS/Preferences/Feeds Tab/SettingsFeeds.m +++ b/baRSS/Preferences/Feeds Tab/SettingsFeeds.m @@ -181,7 +181,6 @@ static NSString *dragNodeType = @"baRSS-feed-drag"; FeedGroup *fg = [children objectAtIndex:i].representedObject; if (fg.sortIndex != (int32_t)i) fg.sortIndex = (int32_t)i; - NSLog(@"%@ - %d", fg.name, fg.sortIndex); [fg iterateSorted:NO overDescendantFeeds:^(Feed *feed, BOOL *cancel) { [feed calculateAndSetIndexPathString]; }]; @@ -259,13 +258,14 @@ static NSString *dragNodeType = @"baRSS-feed-drag"; BOOL isFeed = (fg.typ == FEED); BOOL isSeperator = (fg.typ == SEPARATOR); BOOL isRefreshColumn = [tableColumn.identifier isEqualToString:@"RefreshColumn"]; + BOOL refreshDisabled = (!isFeed || fg.refreshStr.length == 0 || [fg.refreshStr characterAtIndex:0] == '0'); NSString *cellIdent = (isRefreshColumn ? @"cellRefresh" : (isSeperator ? @"cellSeparator" : @"cellFeed")); // owner is nil to prohibit repeated awakeFromNib calls NSTableCellView *cellView = [self.outlineView makeViewWithIdentifier:cellIdent owner:nil]; if (isRefreshColumn) { - cellView.textField.stringValue = (isFeed && fg.refreshStr.length > 0 ? fg.refreshStr : @""); + cellView.textField.stringValue = (refreshDisabled ? (isFeed ? @"--" : @"") : fg.refreshStr); } else if (isSeperator) { return cellView; // the refresh cell is already skipped with the above if condition } else { @@ -281,10 +281,8 @@ static NSString *dragNodeType = @"baRSS-feed-drag"; cellView.imageView.image = defaultRSSIcon; } } - if (isFeed) {// also for refresh column - BOOL feedDisbaled = (fg.refreshStr.length == 0 || [fg.refreshStr characterAtIndex:0] == '0'); - cellView.textField.textColor = (feedDisbaled ? [NSColor disabledControlTextColor] : [NSColor controlTextColor]); - } + // also for refresh column + cellView.textField.textColor = (isFeed && refreshDisabled ? [NSColor disabledControlTextColor] : [NSColor controlTextColor]); return cellView; } diff --git a/baRSS/Status Bar Menu/BarMenu.m b/baRSS/Status Bar Menu/BarMenu.m index 3a090c2..152e5ab 100644 --- a/baRSS/Status Bar Menu/BarMenu.m +++ b/baRSS/Status Bar Menu/BarMenu.m @@ -89,7 +89,7 @@ } else { self.barItem.title = @""; } - // BOOL hasNet = [FeedDownload isNetworkReachable]; + // BOOL hasNet = [FeedDownload allowNetworkConnection]; if (self.unreadCountTotal > 0 && [UserPrefs defaultYES:@"tintMenuBarIcon"]) { self.barItem.image = [RSSIcon templateIcon:16 tint:[NSColor rssOrange]]; } else { @@ -281,13 +281,14 @@ Insert default menu items for the main menu only. Like 'Pause Updates', 'Update all feeds', 'Preferences' and 'Quit'. */ - (void)insertMainMenuHeader:(NSMenu*)menu { - NSMenuItem *item1 = [NSMenuItem itemWithTitle:NSLocalizedString(@"Pause Updates", nil) - action:@selector(pauseUpdates:) target:self tag:TagPauseUpdates]; + NSMenuItem *item1 = [NSMenuItem itemWithTitle:@"" action:@selector(pauseUpdates:) target:self tag:TagPauseUpdates]; NSMenuItem *item2 = [NSMenuItem itemWithTitle:NSLocalizedString(@"Update all feeds", nil) action:@selector(updateAllFeeds:) target:self tag:TagUpdateFeed]; + item1.title = ([FeedDownload isPaused] ? + NSLocalizedString(@"Resume Updates", nil) : NSLocalizedString(@"Pause Updates", nil)); if ([UserPrefs defaultYES:@"globalUpdateAll"] == NO) item2.hidden = YES; - if (![FeedDownload isNetworkReachable]) + if (![FeedDownload allowNetworkConnection]) item2.enabled = NO; [menu insertItem:item1 atIndex:0]; [menu insertItem:item2 atIndex:1]; @@ -325,22 +326,21 @@ - (void)preferencesClosed:(id)sender { [[NSNotificationCenter defaultCenter] removeObserver:self name:NSWindowWillCloseNotification object:self.prefWindow.window]; self.prefWindow = nil; - [FeedDownload scheduleNextUpdateForced:NO]; + [FeedDownload scheduleUpdateForUpcomingFeeds]; } /** Called when user clicks on 'Pause Updates' in the main menu (only). */ - (void)pauseUpdates:(NSMenuItem*)sender { - NSLog(@"1pause"); + [FeedDownload setPaused:![FeedDownload isPaused]]; } /** Called when user clicks on 'Update all feeds' in the main menu (only). */ - (void)updateAllFeeds:(NSMenuItem*)sender { - // TODO: Disable 'update all' menu item during update? - [FeedDownload scheduleNextUpdateForced:YES]; + [FeedDownload forceUpdateAllFeeds]; } /**