Bgerstle has uploaded a new change for review. https://gerrit.wikimedia.org/r/226429
Change subject: fix crash when fetching title with empty text ...................................................................... fix crash when fetching title with empty text pass an error instead of silently failing or crashing Bug: T106026 Change-Id: Ie95e4410097d3597ac7f46da5d054c4334f7e33e --- M Wikipedia.xcodeproj/project.pbxproj M Wikipedia/Networking/Fetchers/MWKLanguageLinkFetcher.h M Wikipedia/Networking/Fetchers/MWKLanguageLinkFetcher.m M Wikipedia/Networking/WMFNetworkUtilities.h A WikipediaUnitTests/MWKLanguageLinkFetcherTests.m M WikipediaUnitTests/WMFAsyncTestCase.h M WikipediaUnitTests/WMFAsyncTestCase.m 7 files changed, 111 insertions(+), 24 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/apps/ios/wikipedia refs/changes/29/226429/1 diff --git a/Wikipedia.xcodeproj/project.pbxproj b/Wikipedia.xcodeproj/project.pbxproj index 8b3b594..ae8fc76 100644 --- a/Wikipedia.xcodeproj/project.pbxproj +++ b/Wikipedia.xcodeproj/project.pbxproj @@ -249,6 +249,7 @@ BC86B9361A92966B00B4C039 /* AFHTTPRequestOperationManager+UniqueRequests.m in Sources */ = {isa = PBXBuildFile; fileRef = BC86B9351A92966B00B4C039 /* AFHTTPRequestOperationManager+UniqueRequests.m */; }; BC86B93D1A929CC500B4C039 /* UICollectionViewFlowLayout+NSCopying.m in Sources */ = {isa = PBXBuildFile; fileRef = BC86B93C1A929CC500B4C039 /* UICollectionViewFlowLayout+NSCopying.m */; }; BC86B9401A929D7900B4C039 /* UICollectionViewFlowLayout+WMFItemSizeThatFits.m in Sources */ = {isa = PBXBuildFile; fileRef = BC86B93F1A929D7900B4C039 /* UICollectionViewFlowLayout+WMFItemSizeThatFits.m */; }; + BC905BD01B60391F0010227E /* MWKLanguageLinkFetcherTests.m in Sources */ = {isa = PBXBuildFile; fileRef = BC905BCF1B60391F0010227E /* MWKLanguageLinkFetcherTests.m */; }; BC90C4BE1AC219FE009F36D2 /* UIWindow+WMFMainScreenWindow.m in Sources */ = {isa = PBXBuildFile; fileRef = BC90C4BD1AC219FE009F36D2 /* UIWindow+WMFMainScreenWindow.m */; }; BC92A7711AFA841C003C4212 /* MWKSection+WMFSharing.m in Sources */ = {isa = PBXBuildFile; fileRef = BC92A7701AFA841C003C4212 /* MWKSection+WMFSharing.m */; }; BC92A7731AFA88D3003C4212 /* MWKSection+WMFSharingTests.m in Sources */ = {isa = PBXBuildFile; fileRef = BC92A7721AFA88D3003C4212 /* MWKSection+WMFSharingTests.m */; }; @@ -809,6 +810,7 @@ BC86B93C1A929CC500B4C039 /* UICollectionViewFlowLayout+NSCopying.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "UICollectionViewFlowLayout+NSCopying.m"; sourceTree = "<group>"; }; BC86B93E1A929D7900B4C039 /* UICollectionViewFlowLayout+WMFItemSizeThatFits.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "UICollectionViewFlowLayout+WMFItemSizeThatFits.h"; sourceTree = "<group>"; }; BC86B93F1A929D7900B4C039 /* UICollectionViewFlowLayout+WMFItemSizeThatFits.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "UICollectionViewFlowLayout+WMFItemSizeThatFits.m"; sourceTree = "<group>"; }; + BC905BCF1B60391F0010227E /* MWKLanguageLinkFetcherTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MWKLanguageLinkFetcherTests.m; sourceTree = "<group>"; }; BC90C4BC1AC219FE009F36D2 /* UIWindow+WMFMainScreenWindow.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "UIWindow+WMFMainScreenWindow.h"; sourceTree = "<group>"; }; BC90C4BD1AC219FE009F36D2 /* UIWindow+WMFMainScreenWindow.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "UIWindow+WMFMainScreenWindow.m"; sourceTree = "<group>"; }; BC92A76E1AFA83D2003C4212 /* WMFSharing.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = WMFSharing.h; path = ShareCard/WMFSharing.h; sourceTree = "<group>"; }; @@ -2146,6 +2148,7 @@ BC5FE5741B1DFF5400273BC0 /* ArticleFetcherTests.m */, BC092BA61B19189100093C59 /* MWKSiteInfoFetcherTests.m */, BC7E4A511B34B53E00EECD8B /* MWKLanguageLinkControllerTests.m */, + BC905BCF1B60391F0010227E /* MWKLanguageLinkFetcherTests.m */, ); name = Tests; path = WikipediaUnitTests; @@ -2985,6 +2988,7 @@ BC23759E1AB8928600B0BAA8 /* WMFDateFormatterTests.m in Sources */, BC0FED771AAA026C002488D7 /* WMFImageURLParsingTests.m in Sources */, BCEC778F1AC9AEC800D9DDA5 /* MWKImage+AssociationTestUtils.m in Sources */, + BC905BD01B60391F0010227E /* MWKLanguageLinkFetcherTests.m in Sources */, 04F1226A1ACB822D002FC3B5 /* NSString+FormattedAttributedStringTests.m in Sources */, BCCED2D01AE03BE20094EB7E /* MWKSectionListTests.m in Sources */, BC7E4A521B34B53E00EECD8B /* MWKLanguageLinkControllerTests.m in Sources */, diff --git a/Wikipedia/Networking/Fetchers/MWKLanguageLinkFetcher.h b/Wikipedia/Networking/Fetchers/MWKLanguageLinkFetcher.h index a7dc68a..d92ed46 100644 --- a/Wikipedia/Networking/Fetchers/MWKLanguageLinkFetcher.h +++ b/Wikipedia/Networking/Fetchers/MWKLanguageLinkFetcher.h @@ -23,8 +23,6 @@ @interface MWKLanguageLinkFetcher : FetcherBase -@property (strong, nonatomic, readonly) MWKTitle* title; - /// Fetches the language links for the given page title. - (instancetype)initAndFetchLanguageLinksForPageTitle:(MWKTitle*)title withManager:(AFHTTPRequestOperationManager*)manager diff --git a/Wikipedia/Networking/Fetchers/MWKLanguageLinkFetcher.m b/Wikipedia/Networking/Fetchers/MWKLanguageLinkFetcher.m index 952b51c..9ca0eef 100644 --- a/Wikipedia/Networking/Fetchers/MWKLanguageLinkFetcher.m +++ b/Wikipedia/Networking/Fetchers/MWKLanguageLinkFetcher.m @@ -13,7 +13,6 @@ @interface MWKLanguageLinkFetcher () -@property (readwrite, strong, nonatomic) MWKTitle* title; @property (strong, nonatomic) AFHTTPRequestOperationManager* manager; @end @@ -29,26 +28,39 @@ } - (instancetype)initWithManager:(AFHTTPRequestOperationManager*)manager delegate:(id<FetchFinishedDelegate>)delegate { + NSParameterAssert(manager); self = [super init]; if (self) { self.manager = manager; self.fetchFinishedDelegate = delegate; - NSAssert([manager.responseSerializer isKindOfClass:[MWKLanguageLinkResponseSerializer class]], - @"%@ needs to have an instance of %@ as its response serializer", - self, [MWKLanguageLinkResponseSerializer class]); } return self; +} + +- (void)finishWithError:(NSError*)error fetchedData:(id)fetchedData block:(void (^)(id))block { + [super finishWithError:error fetchedData:fetchedData]; + if (block) { + dispatchOnMainQueue(^{ + block(error ? : fetchedData); + }); + } } - (void)fetchLanguageLinksForTitle:(MWKTitle*)title success:(void (^)(NSArray*))success failure:(void (^)(NSError*))failure { - self.title = title; - NSURL* url = [[SessionSingleton sharedInstance] urlForLanguage:self.title.site.language]; + if (!title.text.length) { + NSError* error = [NSError errorWithDomain:WMFNetworkingErrorDomain + code:WMFNetworkingError_InvalidParameters + userInfo:nil]; + [self finishWithError:error fetchedData:nil block:failure]; + return; + } + NSURL* url = [[SessionSingleton sharedInstance] urlForLanguage:title.site.language]; NSDictionary* params = @{ @"action": @"query", @"prop": @"langlinks", - @"titles": self.title.text, + @"titles": title.text, @"lllimit": @"500", @"llprop": WMFJoinedPropertyParameters(@[@"langname", @"autonym"]), @"llinlanguagecode": [[NSLocale currentLocale] objectForKey:NSLocaleLanguageCode], @@ -60,24 +72,14 @@ parameters:params success:^(AFHTTPRequestOperation* operation, NSDictionary* indexedLanguageLinks) { [[MWNetworkActivityIndicatorManager sharedManager] pop]; - NSAssert([[indexedLanguageLinks allValues] firstObject], + NSArray* languageLinksForTitle = [[indexedLanguageLinks allValues] firstObject]; + NSAssert(languageLinksForTitle, @"Expected language links to return one object for the title we fetched, but got: %@", indexedLanguageLinks); - NSArray* languageLinksForTitle = [[indexedLanguageLinks allValues] firstObject]; - if (success) { - dispatch_async(dispatch_get_main_queue(), ^{ - success(languageLinksForTitle); - }); - } - [self finishWithError:nil fetchedData:languageLinksForTitle]; + [self finishWithError:nil fetchedData:languageLinksForTitle block:success]; } failure:^(AFHTTPRequestOperation* operation, NSError* error) { [[MWNetworkActivityIndicatorManager sharedManager] pop]; - if (error) { - dispatch_async(dispatch_get_main_queue(), ^{ - failure(error); - }); - } - [self finishWithError:error fetchedData:nil]; + [self finishWithError:error fetchedData:nil block:failure]; }]; } diff --git a/Wikipedia/Networking/WMFNetworkUtilities.h b/Wikipedia/Networking/WMFNetworkUtilities.h index 821a16b..0caf472 100644 --- a/Wikipedia/Networking/WMFNetworkUtilities.h +++ b/Wikipedia/Networking/WMFNetworkUtilities.h @@ -13,7 +13,8 @@ extern NSString* const WMFNetworkingErrorDomain; typedef NS_ENUM (NSInteger, WMFNetworkingError) { - WMFNetworkingError_APIError + WMFNetworkingError_APIError, + WMFNetworkingError_InvalidParameters }; diff --git a/WikipediaUnitTests/MWKLanguageLinkFetcherTests.m b/WikipediaUnitTests/MWKLanguageLinkFetcherTests.m new file mode 100644 index 0000000..6d6dfc1 --- /dev/null +++ b/WikipediaUnitTests/MWKLanguageLinkFetcherTests.m @@ -0,0 +1,76 @@ +// +// MWKLanguageLinkFetcherTests.m +// Wikipedia +// +// Created by Brian Gerstle on 7/22/15. +// Copyright (c) 2015 Wikimedia Foundation. All rights reserved. +// + +#import <UIKit/UIKit.h> +#import <XCTest/XCTest.h> +#import "WMFAsyncTestCase.h" +#import "MWKLanguageLinkFetcher.h" +#import "WMFNetworkUtilities.h" +#import <AFNetworking/AFHTTPRequestOperationManager.h> + +#define MOCKITO_SHORTHAND 1 +#import <OCMockito/OCMockito.h> + +#define HC_SHORTHAND 1 +#import <OCHamcrest/OCHamcrest.h> + +@interface MWKLanguageLinkFetcherTests : WMFAsyncTestCase +@property (nonatomic, strong) AFHTTPRequestOperationManager* mockManager; +@property (nonatomic, strong) id<FetchFinishedDelegate> mockDelegate; +@property (nonatomic, strong) MWKLanguageLinkFetcher* fetcher; +@end + +@implementation MWKLanguageLinkFetcherTests + +- (void)setUp { + self.mockDelegate = mockProtocol(@protocol(FetchFinishedDelegate)); + self.mockManager = mock([AFHTTPRequestOperationManager class]); + self.fetcher = [[MWKLanguageLinkFetcher alloc] initWithManager:self.mockManager + delegate:self.mockDelegate]; + + [super setUp]; +} + +- (void)testFetchingNilTitle { + PushExpectation(); + [self.fetcher fetchLanguageLinksForTitle:nil success:^(NSArray* langLinks) { + XCTFail(@"Expected nil title to result in failure."); + } failure:^(NSError* error) { + XCTAssertEqual(error.code, WMFNetworkingError_InvalidParameters); + [self popExpectationAfter:nil]; + }]; + WaitForExpectations(); + [[MKTVerify(self.mockDelegate) withMatcher:equalTo(@(FETCH_FINAL_STATUS_FAILED)) + forArgument:2] + fetchFinished:self.fetcher + fetchedData:nil + status:0 + error:[NSError errorWithDomain:WMFNetworkingErrorDomain code:WMFNetworkingError_InvalidParameters userInfo:nil]]; +} + +- (void)testFetchingEmptyTitle { + MWKTitle* mockTitle = mock([MWKTitle class]); + [given([mockTitle text]) willReturn:@""]; + + PushExpectation(); + [self.fetcher fetchLanguageLinksForTitle:mockTitle success:^(NSArray* langLinks) { + XCTFail(@"Expected empty title to result in failure."); + } failure:^(NSError* error) { + XCTAssertEqual(error.code, WMFNetworkingError_InvalidParameters); + [self popExpectationAfter:nil]; + }]; + WaitForExpectations(); + [[MKTVerify(self.mockDelegate) withMatcher:equalTo(@(FETCH_FINAL_STATUS_FAILED)) + forArgument:2] + fetchFinished:self.fetcher + fetchedData:nil + status:0 + error:[NSError errorWithDomain:WMFNetworkingErrorDomain code:WMFNetworkingError_InvalidParameters userInfo:nil]]; +} + +@end diff --git a/WikipediaUnitTests/WMFAsyncTestCase.h b/WikipediaUnitTests/WMFAsyncTestCase.h index f1f8320..744f1fd 100644 --- a/WikipediaUnitTests/WMFAsyncTestCase.h +++ b/WikipediaUnitTests/WMFAsyncTestCase.h @@ -16,6 +16,8 @@ @interface WMFAsyncTestCase : XCTestCase +- (void)popExpectation; + - (void)popExpectationAfter:(dispatch_block_t)block; - (void)pushExpectation:(const char*)file line:(int)line; diff --git a/WikipediaUnitTests/WMFAsyncTestCase.m b/WikipediaUnitTests/WMFAsyncTestCase.m index b5270f9..f9aa481 100644 --- a/WikipediaUnitTests/WMFAsyncTestCase.m +++ b/WikipediaUnitTests/WMFAsyncTestCase.m @@ -42,4 +42,8 @@ [expectation fulfill]; } +- (void)popExpectation { + [self popExpectationAfter:nil]; +} + @end -- To view, visit https://gerrit.wikimedia.org/r/226429 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie95e4410097d3597ac7f46da5d054c4334f7e33e Gerrit-PatchSet: 1 Gerrit-Project: apps/ios/wikipedia Gerrit-Branch: master Gerrit-Owner: Bgerstle <bgers...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits