Skip to content

Parity: NSCoding: ISO8601DateFormatter #2167

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

Merged
merged 3 commits into from
Apr 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
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
40 changes: 38 additions & 2 deletions Foundation/ISO8601DateFormatter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,44 @@ open class ISO8601DateFormatter : Formatter, NSSecureCoding {
super.init()
}

public required init?(coder aDecoder: NSCoder) { NSUnimplemented() }
open override func encode(with aCoder: NSCoder) { NSUnimplemented() }
public required init?(coder aDecoder: NSCoder) {
guard aDecoder.allowsKeyedCoding else {
fatalError("Decoding ISO8601DateFormatter requires a coder that allows keyed coding")
}

self.formatOptions = Options(rawValue: UInt(aDecoder.decodeInteger(forKey: "NS.formatOptions")))

let timeZone: NSTimeZone?

if aDecoder.containsValue(forKey: "NS.timeZone") {
if let tz = aDecoder.decodeObject(of: NSTimeZone.self, forKey: "NS.timeZone") {
timeZone = tz
Copy link

@jrtibbetts jrtibbetts Apr 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be clearer to keep all the time zone-related statements together. I would just make it

if let tz = aDecoder.decodeObject(of: NSTimeZone.self,
                                  forKey: "NS.timeZone")?._swiftObject {
    self.timeZone = tz
}

Then you can do away with lines 88 (let timeZone: NSTimeZone?), 97-99, and 102-105 altogether.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's too many things to parse in a single line, I feel.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. But can they be grouped together a little more?

} else {
aDecoder.failWithError(CocoaError(.coderReadCorrupt, userInfo: [ NSLocalizedDescriptionKey: "Time zone was corrupt while decoding ISO8601DateFormatter" ]))
return nil
}
} else {
timeZone = nil
}

if let zone = timeZone?._swiftObject {
self.timeZone = zone
}

super.init()
}

open override func encode(with aCoder: NSCoder) {
guard aCoder.allowsKeyedCoding else {
fatalError("Encoding ISO8601DateFormatter requires a coder that allows keyed coding")
}

aCoder.encode(Int(formatOptions.rawValue), forKey: "NS.formatOptions")
if let timeZone = timeZone {
aCoder.encode(timeZone._nsObject, forKey: "NS.timeZone")
}
}

public static var supportsSecureCoding: Bool { return true }

open func string(from date: Date) -> String {
Expand Down
22 changes: 20 additions & 2 deletions TestFoundation/FixtureValues.swift
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ enum Fixtures {
}

static let dateIntervalFormatterValuesSetWithoutTemplate = TypedFixture<DateIntervalFormatter>("DateIntervalFormatter-ValuesSetWithoutTemplate") {

let dif = DateIntervalFormatter()

var calendar = Calendar.neutral
Expand All @@ -111,7 +110,6 @@ enum Fixtures {
}

static let dateIntervalFormatterValuesSetWithTemplate = TypedFixture<DateIntervalFormatter>("DateIntervalFormatter-ValuesSetWithTemplate") {

let dif = DateIntervalFormatter()

var calendar = Calendar.neutral
Expand All @@ -126,6 +124,24 @@ enum Fixtures {
return dif
}

// ===== ISO8601DateFormatter =====

static let iso8601FormatterDefault = TypedFixture<ISO8601DateFormatter>("ISO8601DateFormatter-Default") {
let idf = ISO8601DateFormatter()
idf.timeZone = Calendar.neutral.timeZone

return idf
}

static let iso8601FormatterOptionsSet = TypedFixture<ISO8601DateFormatter>("ISO8601DateFormatter-OptionsSet") {
let idf = ISO8601DateFormatter()
idf.timeZone = Calendar.neutral.timeZone

idf.formatOptions = [ .withDay, .withWeekOfYear, .withMonth, .withTimeZone, .withColonSeparatorInTimeZone, .withDashSeparatorInDate ]

return idf
}

// ===== Fixture list =====

static let _listOfAllFixtures: [AnyFixture] = [
Expand All @@ -136,6 +152,8 @@ enum Fixtures {
AnyFixture(Fixtures.dateIntervalFormatterDefault),
AnyFixture(Fixtures.dateIntervalFormatterValuesSetWithTemplate),
AnyFixture(Fixtures.dateIntervalFormatterValuesSetWithoutTemplate),
AnyFixture(Fixtures.iso8601FormatterDefault),
AnyFixture(Fixtures.iso8601FormatterOptionsSet),
]

// This ensures that we do not have fixtures with duplicate identifiers:
Expand Down
Binary file not shown.
Binary file not shown.
41 changes: 32 additions & 9 deletions TestFoundation/TestISO8601DateFormatter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,6 @@

class TestISO8601DateFormatter: XCTestCase {

static var allTests : [(String, (TestISO8601DateFormatter) -> () throws -> Void)] {

return [
("test_stringFromDate", test_stringFromDate),
("test_dateFromString", test_dateFromString),
("test_stringFromDateClass", test_stringFromDateClass),
]
}

func test_stringFromDate() {
let formatter = DateFormatter()
formatter.dateFormat = "yyyy/MM/dd HH:mm:ss.SSSS zzz"
Expand Down Expand Up @@ -318,4 +309,36 @@ class TestISO8601DateFormatter: XCTestCase {
#endif
}

let fixtures = [
Fixtures.iso8601FormatterDefault,
Fixtures.iso8601FormatterOptionsSet
]

func areEqual(_ a: ISO8601DateFormatter, _ b: ISO8601DateFormatter) -> Bool {
return a.formatOptions == b.formatOptions &&
a.timeZone.identifier == b.timeZone.identifier
}

func test_codingRoundtrip() throws {
for fixture in fixtures {
try fixture.assertValueRoundtripsInCoder(secureCoding: true, matchingWith: areEqual(_:_:))
}
}

func test_loadingFixtures() throws {
for fixture in fixtures {
try fixture.assertLoadedValuesMatch(areEqual(_:_:))
}
}

static var allTests : [(String, (TestISO8601DateFormatter) -> () throws -> Void)] {

return [
("test_stringFromDate", test_stringFromDate),
("test_dateFromString", test_dateFromString),
("test_stringFromDateClass", test_stringFromDateClass),
("test_codingRoundtrip", test_codingRoundtrip),
("test_loadingFixtures", test_loadingFixtures),
]
}
}
41 changes: 41 additions & 0 deletions TestFoundation/Utilities.swift
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,47 @@ extension Fixture where ValueType: NSObject & NSCoding {
func loadEach(handler: (ValueType, FixtureVariant) throws -> Void) throws {
try self.loadEach(fixtureRepository: try testBundle().url(forResource: "Fixtures", withExtension: nil).unwrapped(), handler: handler)
}

func assertLoadedValuesMatch(_ matchHandler: (ValueType, ValueType) -> Bool = { $0 == $1 }) throws {
let reference = try make()
try loadEach(handler: { (value, variant) in
XCTAssertTrue(matchHandler(reference, value), "The fixture with identifier \(identifier) failed to match for on-disk variant \(variant)")
})
}

func assertValueRoundtripsInCoder(settingUpArchiverWith archiverSetup: (NSKeyedArchiver) -> Void = { _ in}, unarchiverWith unarchiverSetup: (NSKeyedUnarchiver) -> Void = { _ in}, matchingWith: (ValueType, ValueType) -> Bool = { $0 == $1 }) throws {
let original = try make()

let coder = NSKeyedArchiver(forWritingWith: NSMutableData())
coder.decodingFailurePolicy = .setErrorAndReturn
archiverSetup(coder)

coder.encode(original, forKey: NSKeyedArchiveRootObjectKey)
coder.finishEncoding()

let data = coder.encodedData

let decoder = NSKeyedUnarchiver(forReadingWith: data)
decoder.decodingFailurePolicy = .setErrorAndReturn
unarchiverSetup(decoder)

let object = decoder.decodeObject(of: ValueType.self, forKey: NSKeyedArchiveRootObjectKey)

XCTAssertNil(decoder.error)
if let object = object {
XCTAssertTrue(matchingWith(object, original), "The fixture with identifier '\(identifier)' failed to match after an in-memory roundtrip.")
} else {
XCTFail("The fixture with identifier '\(identifier)' failed to decode after an in-memory roundtrip.")
}
}

func assertValueRoundtripsInCoder(secureCoding: Bool, matchingWith: (ValueType, ValueType) -> Bool = { $0 == $1 }) throws {
try assertValueRoundtripsInCoder(settingUpArchiverWith: { (archiver) in
archiver.requiresSecureCoding = secureCoding
}, unarchiverWith: { (unarchiver) in
unarchiver.requiresSecureCoding = secureCoding
}, matchingWith: matchingWith)
}
}

/// Test that the elements of `instances` satisfy the semantic
Expand Down