Mhurd has submitted this change and it was merged.
Change subject: Fix NSDateFormatter issues
......................................................................
Fix NSDateFormatter issues
We were failing to parse timestamps when the device was set to Arabic on
iOS 6. Also, the main date formatter wasn't thread-safe.
- Made main date formatter thread safe
- Refactored the "short time" formatter w/ isMainThread assertion
- Added unit tests for formatter validity across all locales and
expected output
- Tweak in "main.m" which cuts off app start during unit testing (faster, more
reliable tests)
Change-Id: Ibc9758578cb370565310140166c6a20d2781e273
---
M Wikipedia.xcodeproj/project.pbxproj
A WikipediaUnitTests/WMFDateFormatterTests.m
M wikipedia/Categories/NSDateFormatter+WMFExtensions.h
M wikipedia/Categories/NSDateFormatter+WMFExtensions.m
M wikipedia/View Controllers/PageHistory/PageHistoryResultCell.m
M wikipedia/View Controllers/PageHistory/PageHistoryViewController.m
M wikipedia/main.m
7 files changed, 137 insertions(+), 47 deletions(-)
Approvals:
Fjalapeno: Looks good to me, but someone else must approve
Mhurd: Looks good to me, approved
jenkins-bot: Verified
diff --git a/Wikipedia.xcodeproj/project.pbxproj
b/Wikipedia.xcodeproj/project.pbxproj
index 71214a4..ce94344 100644
--- a/Wikipedia.xcodeproj/project.pbxproj
+++ b/Wikipedia.xcodeproj/project.pbxproj
@@ -233,6 +233,7 @@
BC0FED751AAA026C002488D7 /* NSArray+BKIndexTests.m in Sources
*/ = {isa = PBXBuildFile; fileRef = BCB58F7B1A8D0C8E00465627 /*
NSArray+BKIndexTests.m */; };
BC0FED761AAA026C002488D7 /* NSString+WMFHTMLParsingTests.m in
Sources */ = {isa = PBXBuildFile; fileRef = C983151B1AA5205700E25EE1 /*
NSString+WMFHTMLParsingTests.m */; };
BC0FED771AAA026C002488D7 /* WMFImageURLParsingTests.m in
Sources */ = {isa = PBXBuildFile; fileRef = BCBDE0AB1AA76EAC006BD29A /*
WMFImageURLParsingTests.m */; };
+ BC23759E1AB8928600B0BAA8 /* WMFDateFormatterTests.m in Sources
*/ = {isa = PBXBuildFile; fileRef = BC23759D1AB8928600B0BAA8 /*
WMFDateFormatterTests.m */; };
BC2CBB8E1AA10F400079A313 /* UIView+WMFFrameUtils.m in Sources
*/ = {isa = PBXBuildFile; fileRef = BC2CBB8D1AA10F400079A313 /*
UIView+WMFFrameUtils.m */; };
BC50C37F1A83C784006DC7AF /* WMFNetworkUtilities.m in Sources */
= {isa = PBXBuildFile; fileRef = BC50C37E1A83C784006DC7AF /*
WMFNetworkUtilities.m */; };
BC50C3871A83CBDA006DC7AF /* MWKImageInfoResponseSerializer.m in
Sources */ = {isa = PBXBuildFile; fileRef = BC50C3861A83CBDA006DC7AF /*
MWKImageInfoResponseSerializer.m */; };
@@ -725,6 +726,7 @@
17A2F22335C5256576CEDBDD /*
Pods-WikipediaUnitTests.release.xcconfig */ = {isa = PBXFileReference;
includeInIndex = 1; lastKnownFileType = text.xcconfig; name =
"Pods-WikipediaUnitTests.release.xcconfig"; path = "Pods/Target Support
Files/Pods-WikipediaUnitTests/Pods-WikipediaUnitTests.release.xcconfig";
sourceTree = "<group>"; };
357504E50DA104E39C6ACFEB /* Pods.release.xcconfig */ = {isa =
PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name =
Pods.release.xcconfig; path = "Pods/Target Support
Files/Pods/Pods.release.xcconfig"; sourceTree = "<group>"; };
8CE61C6963F825760822A28A /* libPods-WikipediaUnitTests.a */ =
{isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0;
path = "libPods-WikipediaUnitTests.a"; sourceTree = BUILT_PRODUCTS_DIR; };
+ BC23759D1AB8928600B0BAA8 /* WMFDateFormatterTests.m */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path
= WMFDateFormatterTests.m; sourceTree = "<group>"; };
BC2CBB8C1AA10F400079A313 /* UIView+WMFFrameUtils.h */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path =
"UIView+WMFFrameUtils.h"; sourceTree = "<group>"; };
BC2CBB8D1AA10F400079A313 /* UIView+WMFFrameUtils.m */ = {isa =
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path
= "UIView+WMFFrameUtils.m"; sourceTree = "<group>"; };
BC4273521A7C736800068882 /* WikipediaUnitTests.xctest */ = {isa
= PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0;
path = WikipediaUnitTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; };
@@ -1986,6 +1988,7 @@
BCBDE0AB1AA76EAC006BD29A /*
WMFImageURLParsingTests.m */,
BCB8487A1AAAADF90077EC24 /*
WMFRoundingUtilitiesTests.m */,
BCDB75C31AB0E8300005593F /*
WMFSubstringUtilsTests.m */,
+ BC23759D1AB8928600B0BAA8 /*
WMFDateFormatterTests.m */,
);
path = WikipediaUnitTests;
sourceTree = "<group>";
@@ -2816,6 +2819,7 @@
BC0FED731AAA026C002488D7 /*
NSMutableDictionary+MaybeSetTests.m in Sources */,
048830D31AB775E3005BF3A1 /*
UIScrollView+WMFScrollsToTop.m in Sources */,
BC0FED6E1AAA0268002488D7 /* MWKImageListTests.m
in Sources */,
+ BC23759E1AB8928600B0BAA8 /*
WMFDateFormatterTests.m in Sources */,
04D686F51AB2949C0009B44A /* MenuButton.m in
Sources */,
BC0FED701AAA026C002488D7 /*
NSArray+PredicateTests.m in Sources */,
04D686F91AB2949C0009B44A /* PaddedLabel.m in
Sources */,
@@ -3309,6 +3313,7 @@
buildSettings = {
BUNDLE_LOADER = "$(TEST_HOST)";
CLANG_WARN_UNREACHABLE_CODE = YES;
+ CODE_SIGN_IDENTITY = "iPhone Developer";
ENABLE_STRICT_OBJC_MSGSEND = YES;
FRAMEWORK_SEARCH_PATHS = (
"$(SDKROOT)/Developer/Library/Frameworks",
@@ -3334,6 +3339,7 @@
buildSettings = {
BUNDLE_LOADER = "$(TEST_HOST)";
CLANG_WARN_UNREACHABLE_CODE = YES;
+ CODE_SIGN_IDENTITY = "iPhone Developer";
ENABLE_STRICT_OBJC_MSGSEND = YES;
FRAMEWORK_SEARCH_PATHS = (
"$(SDKROOT)/Developer/Library/Frameworks",
diff --git a/WikipediaUnitTests/WMFDateFormatterTests.m
b/WikipediaUnitTests/WMFDateFormatterTests.m
new file mode 100644
index 0000000..bfbb7da
--- /dev/null
+++ b/WikipediaUnitTests/WMFDateFormatterTests.m
@@ -0,0 +1,67 @@
+//
+// WMFDateFormatterTests.m
+// Wikipedia
+//
+// Created by Brian Gerstle on 3/17/15.
+// Copyright (c) 2015 Wikimedia Foundation. All rights reserved.
+//
+
+#import <UIKit/UIKit.h>
+#import <XCTest/XCTest.h>
+#import "NSDateFormatter+WMFExtensions.h"
+
+#define HC_SHORTHAND 1
+#import <OCHamcrest/OCHamcrest.h>
+
+@interface WMFDateFormatterTests : XCTestCase
+
+@end
+
+@implementation WMFDateFormatterTests
+
+- (void)testIso8601Example {
+ NSString* testTimestamp = @"2015-02-10T10:31:27Z";
+ NSDate* decodedDate = [[NSDateFormatter wmf_iso8601Formatter]
dateFromString:testTimestamp];
+ assertThat(decodedDate, is(notNilValue()));
+ assertThat([[NSDateFormatter wmf_iso8601Formatter]
stringFromDate:decodedDate], is(equalTo(testTimestamp)));
+}
+
+- (void)testShortTimeFormatterIsValidForAllLocales {
+ NSString* testTimestamp = @"2015-02-10T10:31:27Z";
+ NSArray* availableLocaleIDs = [NSLocale availableLocaleIdentifiers];
+
+ dispatch_apply(availableLocaleIDs.count,
dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0), ^(size_t i) {
+ NSString* localeID = availableLocaleIDs[i];
+ NSLocale* locale = [NSLocale localeWithLocaleIdentifier:localeID];
+
+ // need to parse date using the "regular" formatter
+ NSDate* decodedDate = [[NSDateFormatter wmf_iso8601Formatter]
dateFromString:testTimestamp];
+ NSParameterAssert(decodedDate);
+
+ // TODO: check for "AM" for corresponding time locales
+ assertThat([[NSDateFormatter wmf_shortTimeFormatterWithLocale:locale]
stringFromDate:decodedDate],
+ describedAs(@"expected non-nil for locale: %0 from
timestamp %1",
+ notNilValue(),
+ localeID,
+ testTimestamp,
+ nil));
+ });
+}
+
+- (void)testShortTimeFormatterExamples {
+ NSString* testTimestamp = @"2015-02-10T14:31:27Z";
+ NSDate* decodedDate = [[NSDateFormatter wmf_iso8601Formatter]
dateFromString:testTimestamp];
+ NSParameterAssert(decodedDate);
+
+ NSDateFormatter* usFormatter =
+ [NSDateFormatter wmf_shortTimeFormatterWithLocale:[NSLocale
localeWithLocaleIdentifier:@"en_US"]];
+
+ NSDateFormatter* gbFormatter =
+ [NSDateFormatter wmf_shortTimeFormatterWithLocale:[NSLocale
localeWithLocaleIdentifier:@"en_GB"]];
+
+ assertThat([usFormatter stringFromDate:decodedDate], is(equalTo(@"2:31
PM")));
+
+ assertThat([gbFormatter stringFromDate:decodedDate],
is(equalTo(@"14:31")));
+}
+
+@end
diff --git a/wikipedia/Categories/NSDateFormatter+WMFExtensions.h
b/wikipedia/Categories/NSDateFormatter+WMFExtensions.h
index 28d54a7..7815191 100644
--- a/wikipedia/Categories/NSDateFormatter+WMFExtensions.h
+++ b/wikipedia/Categories/NSDateFormatter+WMFExtensions.h
@@ -4,10 +4,33 @@
@interface NSDateFormatter (WMFExtensions)
/**
- * Standard formatter. Cached for performance
+ * Formatter which can be used to parse timestamps from the mediawiki API.
*
- * @return The formatter
+ * @note It is safe to call this method from any thread.
+ *
+ * @return Singleton @c NSDateFormatter for transcoding WMF timestamps.
*/
+ (NSDateFormatter*)wmf_iso8601Formatter;
+
+/**
+ * Formatter which can be used to present a short time string for a given date.
+ *
+ * @warning Do not attempt to parse raw timestamps from the mediawiki API
using this method. Use the unstyled
+ * @c +wmf_iso8601Formatter method instead.
+ *
+ * @note This method is not thread safe, as it is intended to only be used
by code which presents text to the user.
+ *
+ * @see +[NSDateFormatter wmf_iso8601Formatter]
+ *
+ * @return Singleton @c NSDateFormatter for displaying times to the user.
+ */
++ (NSDateFormatter*)wmf_shortTimeFormatter;
+
+/**
+ * Create an short-style time formatter with the given locale.
+ * @warning This method is exposed for testing only, use @c
+wmf_shortTimeFormatter instead.
+ */
++ (NSDateFormatter*)wmf_shortTimeFormatterWithLocale:(NSLocale*)locale;
+
@end
diff --git a/wikipedia/Categories/NSDateFormatter+WMFExtensions.m
b/wikipedia/Categories/NSDateFormatter+WMFExtensions.m
index ac21e25..43e7da5 100644
--- a/wikipedia/Categories/NSDateFormatter+WMFExtensions.m
+++ b/wikipedia/Categories/NSDateFormatter+WMFExtensions.m
@@ -1,19 +1,40 @@
#import "NSDateFormatter+WMFExtensions.h"
+static NSString* const WMF_ISO8601_FORMAT =
@"yyyy'-'MM'-'dd'T'HH':'mm':'ss'Z'";
+
@implementation NSDateFormatter (WMFExtensions)
+ (NSDateFormatter*)wmf_iso8601Formatter {
- static NSDateFormatter* _formatter = nil;
+ static NSDateFormatter* iso8601Formatter = nil;
+ static dispatch_once_t onceToken;
+ dispatch_once(&onceToken, ^{
+ // need to use "en" locale, otherwise the timestamp will fail to parse
when the current locale is arabic on iOS 6
+ iso8601Formatter = [NSDateFormatter new];
+ iso8601Formatter.timeZone = [NSTimeZone timeZoneWithName:@"UTC"];
+ iso8601Formatter.dateFormat = WMF_ISO8601_FORMAT;;
+ iso8601Formatter.locale = [NSLocale localeWithLocaleIdentifier:@"en"];
+ });
+ return iso8601Formatter;
+}
- if (!_formatter) {
- // See: https://www.mediawiki.org/wiki/Manual:WfTimestamp
- _formatter = [[NSDateFormatter alloc] init];
- [_formatter setTimeZone:[NSTimeZone timeZoneWithName:@"UTC"]];
- [_formatter setDateFormat:@"yyyy'-'MM'-'dd'T'HH':'mm':'ss'Z'"];
++ (NSDateFormatter*)wmf_shortTimeFormatter {
+ NSParameterAssert([NSThread isMainThread]);
+ static NSDateFormatter* shortTimeFormatter = nil;
+ if (!shortTimeFormatter) {
+ shortTimeFormatter = [self wmf_shortTimeFormatterWithLocale:[NSLocale
currentLocale]];
}
+ return shortTimeFormatter;
+}
- return _formatter;
++ (NSDateFormatter*)wmf_shortTimeFormatterWithLocale:(NSLocale*)locale {
+ NSDateFormatter* shortTimeFormatter = [NSDateFormatter new];
+ shortTimeFormatter.timeZone = [NSTimeZone
timeZoneWithName:@"UTC"];
+ shortTimeFormatter.dateStyle = NSDateFormatterNoStyle;
+ shortTimeFormatter.timeStyle = NSDateFormatterShortStyle;
+ shortTimeFormatter.formatterBehavior = NSDateFormatterBehavior10_4;
+ shortTimeFormatter.locale = locale;
+ return shortTimeFormatter;
}
@end
diff --git a/wikipedia/View Controllers/PageHistory/PageHistoryResultCell.m
b/wikipedia/View Controllers/PageHistory/PageHistoryResultCell.m
index f2d1597..2b928f4 100644
--- a/wikipedia/View Controllers/PageHistory/PageHistoryResultCell.m
+++ b/wikipedia/View Controllers/PageHistory/PageHistoryResultCell.m
@@ -7,6 +7,7 @@
#import "Defines.h"
#import "NSString+Extras.h"
#import "UIFont+WMFStyle.h"
+#import "NSDateFormatter+WMFExtensions.h"
@interface PageHistoryResultCell ()
@@ -21,22 +22,6 @@
@implementation PageHistoryResultCell
-+ (NSDateFormatter*)dateFormatter {
- static NSDateFormatter* _formatter = nil;
-
- if (!_formatter) {
- // See: https://www.mediawiki.org/wiki/Manual:WfTimestamp
- _formatter = [[NSDateFormatter alloc] init];
- [_formatter setTimeZone:[NSTimeZone timeZoneWithName:@"UTC"]];
- [_formatter setDateFormat:@"yyyy'-'MM'-'dd'T'HH':'mm':'ss'Z'"];
- _formatter.dateStyle = NSDateFormatterNoStyle;
- _formatter.timeStyle = NSDateFormatterShortStyle;
- _formatter.formatterBehavior = NSDateFormatterBehavior10_4;
- }
-
- return _formatter;
-}
-
- (void)setName:(NSString*)name
time:(NSString*)time
delta:(NSNumber*)delta
@@ -45,7 +30,7 @@
separator:(BOOL)separator {
self.nameLabel.text = name;
- self.timeLabel.text = [[[self class] dateFormatter]
stringForObjectValue:[time getDateFromIso8601DateString]];
+ self.timeLabel.text = [[NSDateFormatter wmf_shortTimeFormatter]
stringFromDate:[time getDateFromIso8601DateString]];
self.deltaLabel.text =
[NSString stringWithFormat:@"%@%@", (delta.integerValue > 0) ? @"+" :
@"", delta.stringValue];
diff --git a/wikipedia/View Controllers/PageHistory/PageHistoryViewController.m
b/wikipedia/View Controllers/PageHistory/PageHistoryViewController.m
index 8cd3401..d2b0960 100644
--- a/wikipedia/View Controllers/PageHistory/PageHistoryViewController.m
+++ b/wikipedia/View Controllers/PageHistory/PageHistoryViewController.m
@@ -13,6 +13,7 @@
#import "Defines.h"
#import "PaddedLabel.h"
#import "UITableView+DynamicCellHeight.h"
+#import "NSDateFormatter+WMFExtensions.h"
#define TABLE_CELL_ID @"PageHistoryResultCell"
@@ -24,22 +25,6 @@
@end
@implementation PageHistoryViewController
-
-+ (NSDateFormatter*)dateFormatter {
- static NSDateFormatter* _formatter = nil;
-
- if (!_formatter) {
- // See: https://www.mediawiki.org/wiki/Manual:WfTimestamp
- _formatter = [[NSDateFormatter alloc] init];
- [_formatter setTimeZone:[NSTimeZone timeZoneWithName:@"UTC"]];
- [_formatter setDateFormat:@"yyyy'-'MM'-'dd'T'HH':'mm':'ss'Z'"];
- _formatter.dateStyle = NSDateFormatterLongStyle;
- _formatter.timeStyle = NSDateFormatterNoStyle;
- _formatter.formatterBehavior = NSDateFormatterBehavior10_4;
- }
-
- return _formatter;
-}
- (NavBarMode)navBarMode {
return NAVBAR_MODE_X_WITH_LABEL;
@@ -209,9 +194,7 @@
NSNumber* daysAgo = sectionDict[@"daysAgo"];
NSDate* date = [NSDate dateWithDaysBeforeNow:daysAgo.integerValue];
- NSString* formattedDate = [[[self class] dateFormatter]
stringForObjectValue:date];
-
- label.text = formattedDate;
+ label.text = [[NSDateFormatter wmf_shortTimeFormatter]
stringFromDate:date];
[view addSubview:label];
diff --git a/wikipedia/main.m b/wikipedia/main.m
index 202bcd0..798ddc4 100644
--- a/wikipedia/main.m
+++ b/wikipedia/main.m
@@ -5,9 +5,14 @@
#import "AppDelegate.h"
-int main(int argc, char* argv[]){
+int main(int argc, char* argv[]) {
+ // disable app when unit testing to allow tests to run in isolation (w/o
side effects)
+ BOOL const isUnitTesting = NSClassFromString(@"XCTestCase") != nil;
@autoreleasepool {
- return UIApplicationMain(argc, argv, nil,
NSStringFromClass([AppDelegate class]));
+ return UIApplicationMain(argc,
+ argv,
+ nil,
+ isUnitTesting ? nil :
NSStringFromClass([AppDelegate class]));
}
}
--
To view, visit https://gerrit.wikimedia.org/r/197387
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibc9758578cb370565310140166c6a20d2781e273
Gerrit-PatchSet: 3
Gerrit-Project: apps/ios/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Bgerstle <[email protected]>
Gerrit-Reviewer: Dr0ptp4kt <[email protected]>
Gerrit-Reviewer: Fjalapeno <[email protected]>
Gerrit-Reviewer: Mhurd <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
