SPM Prep - Convert "WordPress JSON date" logic from Objective-C to Swift #758
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import Foundation | ||
|
|
||
| extension NSDate { | ||
|
|
||
| @objc | ||
| static let rfc3339DateFormatter = DateFormatter.rfc3339Formatter | ||
|
|
||
| /// Parses a date string | ||
| /// | ||
| /// Dates in the format specified in http://www.w3.org/TR/NOTE-datetime should be OK. | ||
| /// The kind of dates returned by the REST API should match that format, even if the doc promises ISO 8601. | ||
| /// | ||
| /// Parsing the full ISO 8601, or even RFC 3339 is more complex than this, and makes no sense right now. | ||
| /// | ||
| /// - Warning: This method doesn't support fractional seconds or dates with leap seconds (23:59:60 turns into 23:59:00) | ||
| // | ||
| // Needs to be `public` because of the usages in the Objective-C code. | ||
| @objc(dateWithWordPressComJSONString:) | ||
| public static func with(wordPressComJSONString jsonString: String) -> Date? { | ||
| self.rfc3339DateFormatter.date(from: jsonString) | ||
| } | ||
|
|
||
| @objc(WordPressComJSONString) | ||
| public func wordPressComJSONString() -> String { | ||
| NSDate.rfc3339DateFormatter.string(from: self as Date) | ||
| } | ||
| } | ||
|
|
||
| extension DateFormatter { | ||
|
|
||
| static let rfc3339Formatter: DateFormatter = { | ||
| let formatter = DateFormatter() | ||
| formatter.dateFormat = "yyyy'-'MM'-'dd'T'HH':'mm':'ssZ" | ||
| formatter.timeZone = NSTimeZone(forSecondsFromGMT: 0) as TimeZone | ||
| formatter.locale = NSLocale(localeIdentifier: "en_US_POSIX") as Locale | ||
| return formatter | ||
| }() | ||
| } | ||
|
There was a problem hiding this comment. It seemed neater to define this pre-configured |
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,7 +40,7 @@ private func decodePost(from object: AnyObject) async throws -> RemotePost { | |
|
|
||
| private func makeParameters<T: Encodable>(from value: T) throws -> [String: AnyObject] { | ||
| let encoder = JSONEncoder() | ||
| encoder.dateEncodingStrategy = .formatted(NSDate.rfc3339DateFormatter) | ||
|
There was a problem hiding this comment. I forgot about the API breakage concerns when making this a Turns out there is one use of this outside the library, in the WordPress iOS tests. I'll update the code reintroducing the |
||
| let data = try encoder.encode(value) | ||
| let object = try JSONSerialization.jsonObject(with: data) | ||
| guard let dictionary = object as? [String: AnyObject] else { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,7 +50,7 @@ final class CommentServiceRemoteRESTTests: RemoteTestCase, RESTTestable { | |
| XCTAssertEqual(comment.authorEmail, "[email protected]") | ||
| XCTAssertEqual(comment.authorUrl, "author URL") | ||
| XCTAssertEqual(comment.authorIP, "000.0.00.000") | ||
| XCTAssertEqual(comment.date, NSDate.with(wordPressComJSONString: "2021-08-04T07:58:49+00:00")) | ||
| XCTAssertEqual(comment.link, "comment URL") | ||
| XCTAssertEqual(comment.parentID, nil) | ||
| XCTAssertEqual(comment.postID, NSNumber(value: 1)) | ||
|
|
@@ -91,7 +91,7 @@ final class CommentServiceRemoteRESTTests: RemoteTestCase, RESTTestable { | |
| XCTAssertEqual(comment.authorEmail, "[email protected]") | ||
| XCTAssertEqual(comment.authorUrl, "author URL") | ||
| XCTAssertEqual(comment.authorIP, "000.0.00.000") | ||
| XCTAssertEqual(comment.date, NSDate.with(wordPressComJSONString: "2021-08-04T07:58:49+00:00")) | ||
| XCTAssertEqual(comment.link, "comment URL") | ||
| XCTAssertEqual(comment.parentID, nil) | ||
| XCTAssertEqual(comment.postID, NSNumber(value: 1)) | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| @testable import WordPressKit | ||
| import XCTest | ||
|
|
||
| // This is an incomplete test for implementing RFC 3339. | ||
| // It's purpose is to ensure our code "works". | ||
| // | ||
| // See also: | ||
| // | ||
| // - https://datatracker.ietf.org/doc/html/rfc3339 | ||
| class NSDateRFC3339Tests: XCTestCase { | ||
|
|
||
| func testDateFormatterConfiguration() throws { | ||
| let rfc3339Formatter = try XCTUnwrap(NSDate.rfc3339DateFormatter) | ||
|
|
||
| XCTAssertEqual(rfc3339Formatter.timeZone, TimeZone(secondsFromGMT: 0)) | ||
| XCTAssertEqual(rfc3339Formatter.locale, Locale(identifier: "en_US_POSIX")) | ||
| XCTAssertEqual(rfc3339Formatter.dateFormat, "yyyy'-'MM'-'dd'T'HH':'mm':'ssZ") | ||
| } | ||
|
|
||
| func testValidRFC3339DateFromString() { | ||
| XCTAssertEqual( | ||
| NSDate.with(wordPressComJSONString: "2023-03-19T15:00:00Z"), | ||
| Date(timeIntervalSince1970: 1_679_238_000) | ||
| ) | ||
| } | ||
|
|
||
| func testInvalidRFC3339DateFromString() { | ||
| XCTAssertNil(NSDate.with(wordPressComJSONString: "2024-01-01")) | ||
| } | ||
|
|
||
| func testInvalidDateFromString() { | ||
| XCTAssertNil(NSDate.with(wordPressComJSONString: "not a date")) | ||
| } | ||
|
|
||
| func testValidRFC3339StringFromDate() { | ||
| XCTAssertEqual( | ||
| NSDate(timeIntervalSince1970: 1_679_238_000).wordPressComJSONString(), | ||
| // Apparently, NSDateFormatter doesn't offer a way to specify Z vs +0000. | ||
| // This might go all the way back to the ISO 8601 and RFC 3339 specs overlap. | ||
| "2023-03-19T15:00:00+0000" | ||
|
There was a problem hiding this comment. Have you tried to put a single quote There was a problem hiding this comment. BTW, this is one of the reasons I think we should avoid using standard names in function names. It's pretty difficult to ensure conformance. There was a problem hiding this comment. If we are being really strict here, this test case should fail, because the output is not in RFC3339 time format. There was a problem hiding this comment. Thanks @crazytonyli ! I didn't noticed that 🤦♂️ I tried locally and it does the job at returning But ☝️ other tests that deal with API and dates fail. I think maybe we should leave it like this for the moment, and update / investigate later on. After all, the format the code uses right now is the "original" one. What do you think? |
||
| ) | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are going to make breaking changes, I guess we have an option of getting rid of the
NSDateextension above and renaming thisrfc3339Formatterto be something likewordPressComDateFormatter.