Bgerstle has uploaded a new change for review. https://gerrit.wikimedia.org/r/226019
Change subject: fix MWKList KVO and article observation ...................................................................... fix MWKList KVO and article observation Change-Id: I6e02b8a5b82e5daaf9cac5250fa9f835bc495770 --- M MediaWikiKit/MediaWikiKit/MWKList.m M MediaWikiKit/MediaWikiKitTests/MWKListTests.m M Wikipedia/Networking/Fetchers/ArticleFetcher.h M Wikipedia/UI-V5/WMFArticleFetcher.h M Wikipedia/UI-V5/WMFArticleFetcher.m M Wikipedia/UI-V5/WMFArticleViewController.m 6 files changed, 51 insertions(+), 43 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/apps/ios/wikipedia refs/changes/19/226019/1 diff --git a/MediaWikiKit/MediaWikiKit/MWKList.m b/MediaWikiKit/MediaWikiKit/MWKList.m index 02c7c3a..1f2a743 100644 --- a/MediaWikiKit/MediaWikiKit/MWKList.m +++ b/MediaWikiKit/MediaWikiKit/MWKList.m @@ -19,7 +19,7 @@ - (instancetype)init { self = [super init]; if (self) { - self.mutableEntries = [NSMutableArray array]; + _mutableEntries = [NSMutableArray array]; self.entriesByTitle = [NSMutableDictionary dictionary]; } return self; @@ -144,6 +144,10 @@ #pragma mark - KVO +- (NSMutableArray*)mutableEntries { + return [self mutableArrayValueForKey:@"entries"]; +} + - (NSUInteger)countOfEntries { return [_mutableEntries count]; } diff --git a/MediaWikiKit/MediaWikiKitTests/MWKListTests.m b/MediaWikiKit/MediaWikiKitTests/MWKListTests.m index 7e33315..6e4ab34 100644 --- a/MediaWikiKit/MediaWikiKitTests/MWKListTests.m +++ b/MediaWikiKit/MediaWikiKitTests/MWKListTests.m @@ -131,4 +131,21 @@ XCTAssertEqual([list countOfEntries], 0, @"Should have length 0 after removing all"); } +- (void)testKVO { + MWKList* list = [[MWKList alloc] init]; + NSMutableArray* observations = [NSMutableArray new]; + [self.KVOController observe:list + keyPath:WMF_SAFE_KEYPATH(list, entries) + options:0 + block:^(id observer, id object, NSDictionary* change) { + [observations addObject:@[change[NSKeyValueChangeKindKey], change[NSKeyValueChangeIndexesKey]]]; + }]; + [list addEntry:self.testObjects[0]]; + [list removeEntry:self.testObjects[0]]; + XCTAssertEqual(observations.count, 2); + XCTAssertEqualObjects(observations[0], (@[@(NSKeyValueChangeInsertion), [NSIndexSet indexSetWithIndex:0]])); + XCTAssertEqualObjects(observations[1], (@[@(NSKeyValueChangeRemoval), [NSIndexSet indexSetWithIndex:0]])); + [self.KVOController unobserve:list]; +} + @end diff --git a/Wikipedia/Networking/Fetchers/ArticleFetcher.h b/Wikipedia/Networking/Fetchers/ArticleFetcher.h index 343918c..95b1d5f 100644 --- a/Wikipedia/Networking/Fetchers/ArticleFetcher.h +++ b/Wikipedia/Networking/Fetchers/ArticleFetcher.h @@ -15,8 +15,4 @@ completionBlock:(WMFArticleHandler)completion errorBlock:(WMFErrorHandler)errorHandler; - - - - @end diff --git a/Wikipedia/UI-V5/WMFArticleFetcher.h b/Wikipedia/UI-V5/WMFArticleFetcher.h index 72b3ead..10b5a03 100644 --- a/Wikipedia/UI-V5/WMFArticleFetcher.h +++ b/Wikipedia/UI-V5/WMFArticleFetcher.h @@ -13,7 +13,7 @@ - (instancetype)initWithDataStore:(MWKDataStore*)dataStore; -- (AnyPromise*)fetchArticleForPageTitle:(MWKTitle*)pageTitle progress:(WMFProgressHandler)progress; +- (AnyPromise*)fetchArticleForPageTitle:(MWKTitle*)pageTitle progress:(WMFProgressHandler __nullable)progress; @end diff --git a/Wikipedia/UI-V5/WMFArticleFetcher.m b/Wikipedia/UI-V5/WMFArticleFetcher.m index 94faf33..727e9be 100644 --- a/Wikipedia/UI-V5/WMFArticleFetcher.m +++ b/Wikipedia/UI-V5/WMFArticleFetcher.m @@ -37,20 +37,14 @@ return _fetcher; } -- (AnyPromise*)fetchArticleForPageTitle:(MWKTitle*)pageTitle progress:(WMFProgressHandler)progress { +- (AnyPromise*)fetchArticleForPageTitle:(MWKTitle*)pageTitle progress:(WMFProgressHandler __nullable)progress { return [AnyPromise promiseWithResolverBlock:^(PMKResolver resolve) { - AFHTTPRequestOperation* operation = [self.fetcher fetchSectionsForTitle:pageTitle inDataStore:self.dataStore withManager:self.operationManager progressBlock:^(CGFloat completionProgress) { - dispatch_async(dispatch_get_main_queue(), ^{ - if (progress) { - progress(completionProgress); - } - }); - } completionBlock:^(MWKArticle* article) { - resolve(article); - } errorBlock:^(NSError* error) { - resolve(error); - }]; - + AFHTTPRequestOperation* operation = [self.fetcher fetchSectionsForTitle:pageTitle + inDataStore:self.dataStore + withManager:self.operationManager + progressBlock:progress + completionBlock:resolve + errorBlock:resolve]; if (operation == nil) { resolve([NSError wmf_errorWithType:WMFErrorTypeStringMissingParameter userInfo:nil]); } diff --git a/Wikipedia/UI-V5/WMFArticleViewController.m b/Wikipedia/UI-V5/WMFArticleViewController.m index c36d555..df22d8f 100644 --- a/Wikipedia/UI-V5/WMFArticleViewController.m +++ b/Wikipedia/UI-V5/WMFArticleViewController.m @@ -64,6 +64,7 @@ return; } + [self unobserveArticleUpdates]; [[WMFImageController sharedInstance] cancelFetchForURL:[NSURL wmf_optionalURLWithString:[_article bestThumbnailImageURL]]]; _article = article; @@ -75,7 +76,7 @@ DDLogVerbose(@"%lu", [article.sections count]); [self updateUI]; - [self fetchArticleIfNeeded]; + [self observeAndFetchArticleIfNeeded]; } - (void)setMode:(WMFArticleControllerMode)mode animated:(BOOL)animated { @@ -86,7 +87,7 @@ _mode = mode; [self updateUIForMode:mode animated:animated]; - [self fetchArticleIfNeeded]; + [self observeAndFetchArticleIfNeeded]; } - (BOOL)isSaved { @@ -125,28 +126,30 @@ - (void)articleUpdatedWithNotification:(NSNotification*)note { MWKArticle* article = note.userInfo[MWKArticleKey]; if ([self.article.title isEqualToTitle:article.title]) { - dispatchOnMainQueue(^{ - self.article = article; - }); + self.article = article; } } #pragma mark - Article Fetching -- (void)fetchArticleIfNeeded { +- (void)observeAndFetchArticleIfNeeded { if (!self.article) { + // nothing to fetch or observe return; } if (self.mode == WMFArticleControllerModeList) { + // don't update or fetch while in list mode return; } - if ([self.article bestThumbnailImageURL] && [self.article isCached]) { - return; + if ([self.article isCached]) { + // observe immediately + [self observeArticleUpdates]; + } else { + // fetch then observe + [self fetchArticle]; } - - [self fetchArticle]; } - (void)fetchArticle { @@ -154,18 +157,15 @@ } - (void)fetchArticleForTitle:(MWKTitle*)title { - [self unobserveArticleUpdates]; - [self.articleFetcher fetchArticleForPageTitle:title progress:^(CGFloat progress){ - }].then(^(MWKArticle* article){ + [self.articleFetcher fetchArticleForPageTitle:title progress:nil].then(^(MWKArticle* article) { + // re-entry, should result in being article being observed self.article = article; - }).catch(^(NSError* error){ + }).catch(^(NSError* error) { if ([error wmf_isWMFErrorOfType:WMFErrorTypeRedirected]) { [self fetchArticleForTitle:[[error userInfo] wmf_redirectTitle]]; } else { NSLog(@"Article Fetch Error: %@", [error localizedDescription]); } - }).then(^(){ - [self observeArticleUpdates]; }); } @@ -235,18 +235,15 @@ switch (mode) { case WMFArticleControllerModeNormal: { self.tableView.scrollEnabled = YES; + [self observeAndFetchArticleIfNeeded]; + break; } - break; - case WMFArticleControllerModeList: { + default: { [self.tableView setContentOffset:CGPointZero animated:animated]; self.tableView.scrollEnabled = NO; + [self unobserveArticleUpdates]; + break; } - break; - case WMFArticleControllerModePopup: { - [self.tableView setContentOffset:CGPointZero animated:animated]; - self.tableView.scrollEnabled = NO; - } - break; } } -- To view, visit https://gerrit.wikimedia.org/r/226019 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6e02b8a5b82e5daaf9cac5250fa9f835bc495770 Gerrit-PatchSet: 1 Gerrit-Project: apps/ios/wikipedia Gerrit-Branch: 5.0 Gerrit-Owner: Bgerstle <bgers...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits