diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlyAddNewFeedOperationTests.swift b/Frameworks/Account/AccountTests/Feedly/FeedlyAddNewFeedOperationTests.swift index c218e8f07..396cc8783 100644 --- a/Frameworks/Account/AccountTests/Feedly/FeedlyAddNewFeedOperationTests.swift +++ b/Frameworks/Account/AccountTests/Feedly/FeedlyAddNewFeedOperationTests.swift @@ -120,7 +120,7 @@ class FeedlyAddNewFeedOperationTests: XCTestCase { XCTAssert(progress.isComplete) } - func testAddNewFeedSuccess() { + func testAddNewFeedSuccess() throws { guard let folder = getFolderByLoadingInitialContent() else { return } @@ -163,7 +163,7 @@ class FeedlyAddNewFeedOperationTests: XCTestCase { XCTAssert(progress.isComplete) - support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "feedStream", subdirectory: subdirectory) + try support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "feedStream", subdirectory: subdirectory) support.checkUnreadStatuses(in: account, againstIdsInStreamInJSONNamed: "unreadIds", subdirectory: subdirectory, testCase: self) } diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlySendArticleStatusesOperationTests.swift b/Frameworks/Account/AccountTests/Feedly/FeedlySendArticleStatusesOperationTests.swift index d508f6635..c35a71670 100644 --- a/Frameworks/Account/AccountTests/Feedly/FeedlySendArticleStatusesOperationTests.swift +++ b/Frameworks/Account/AccountTests/Feedly/FeedlySendArticleStatusesOperationTests.swift @@ -50,7 +50,8 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { let statuses = articleIds.map { SyncStatus(articleID: $0, key: .read, flag: false) } let insertExpectation = expectation(description: "Inserted Statuses") - container.database.insertStatuses(statuses) { + container.database.insertStatuses(statuses) { error in + XCTAssertNil(error) insertExpectation.fulfill() } @@ -74,7 +75,17 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { waitForExpectations(timeout: 2) - XCTAssertEqual(container.database.selectPendingCount(), 0) + let selectPendingCountExpectation = expectation(description: "Did Select Pending Count") + container.database.selectPendingCount { result in + do { + let statusCount = try result.get() + XCTAssertEqual(statusCount, 0) + selectPendingCountExpectation.fulfill() + } catch { + XCTFail("Error unwrapping database result: \(error)") + } + } + waitForExpectations(timeout: 2) } func testSendUnreadFailure() { @@ -82,7 +93,8 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { let statuses = articleIds.map { SyncStatus(articleID: $0, key: .read, flag: false) } let insertExpectation = expectation(description: "Inserted Statuses") - container.database.insertStatuses(statuses) { + container.database.insertStatuses(statuses) { error in + XCTAssertNil(error) insertExpectation.fulfill() } @@ -106,7 +118,17 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { waitForExpectations(timeout: 2) - XCTAssertEqual(container.database.selectPendingCount(), statuses.count) + let selectPendingCountExpectation = expectation(description: "Did Select Pending Count") + container.database.selectPendingCount { result in + do { + let statusCount = try result.get() + XCTAssertEqual(statusCount, statuses.count) + selectPendingCountExpectation.fulfill() + } catch { + XCTFail("Error unwrapping database result: \(error)") + } + } + waitForExpectations(timeout: 2) } func testSendReadSuccess() { @@ -114,7 +136,8 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { let statuses = articleIds.map { SyncStatus(articleID: $0, key: .read, flag: true) } let insertExpectation = expectation(description: "Inserted Statuses") - container.database.insertStatuses(statuses) { + container.database.insertStatuses(statuses) { error in + XCTAssertNil(error) insertExpectation.fulfill() } @@ -138,7 +161,17 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { waitForExpectations(timeout: 2) - XCTAssertEqual(container.database.selectPendingCount(), 0) + let selectPendingCountExpectation = expectation(description: "Did Select Pending Count") + container.database.selectPendingCount { result in + do { + let statusCount = try result.get() + XCTAssertEqual(statusCount, 0) + selectPendingCountExpectation.fulfill() + } catch { + XCTFail("Error unwrapping database result: \(error)") + } + } + waitForExpectations(timeout: 2) } func testSendReadFailure() { @@ -146,7 +179,8 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { let statuses = articleIds.map { SyncStatus(articleID: $0, key: .read, flag: true) } let insertExpectation = expectation(description: "Inserted Statuses") - container.database.insertStatuses(statuses) { + container.database.insertStatuses(statuses) { error in + XCTAssertNil(error) insertExpectation.fulfill() } @@ -170,7 +204,17 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { waitForExpectations(timeout: 2) - XCTAssertEqual(container.database.selectPendingCount(), statuses.count) + let selectPendingCountExpectation = expectation(description: "Did Select Pending Count") + container.database.selectPendingCount { result in + do { + let statusCount = try result.get() + XCTAssertEqual(statusCount, statuses.count) + selectPendingCountExpectation.fulfill() + } catch { + XCTFail("Error unwrapping database result: \(error)") + } + } + waitForExpectations(timeout: 2) } func testSendStarredSuccess() { @@ -178,7 +222,8 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { let statuses = articleIds.map { SyncStatus(articleID: $0, key: .starred, flag: true) } let insertExpectation = expectation(description: "Inserted Statuses") - container.database.insertStatuses(statuses) { + container.database.insertStatuses(statuses) { error in + XCTAssertNil(error) insertExpectation.fulfill() } @@ -202,7 +247,17 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { waitForExpectations(timeout: 2) - XCTAssertEqual(container.database.selectPendingCount(), 0) + let selectPendingCountExpectation = expectation(description: "Did Select Pending Count") + container.database.selectPendingCount { result in + do { + let statusCount = try result.get() + XCTAssertEqual(statusCount, 0) + selectPendingCountExpectation.fulfill() + } catch { + XCTFail("Error unwrapping database result: \(error)") + } + } + waitForExpectations(timeout: 2) } func testSendStarredFailure() { @@ -210,7 +265,8 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { let statuses = articleIds.map { SyncStatus(articleID: $0, key: .starred, flag: true) } let insertExpectation = expectation(description: "Inserted Statuses") - container.database.insertStatuses(statuses) { + container.database.insertStatuses(statuses) { error in + XCTAssertNil(error) insertExpectation.fulfill() } @@ -234,7 +290,17 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { waitForExpectations(timeout: 2) - XCTAssertEqual(container.database.selectPendingCount(), statuses.count) + let selectPendingCountExpectation = expectation(description: "Did Select Pending Count") + container.database.selectPendingCount { result in + do { + let statusCount = try result.get() + XCTAssertEqual(statusCount, statuses.count) + selectPendingCountExpectation.fulfill() + } catch { + XCTFail("Error unwrapping database result: \(error)") + } + } + waitForExpectations(timeout: 2) } func testSendUnstarredSuccess() { @@ -242,7 +308,8 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { let statuses = articleIds.map { SyncStatus(articleID: $0, key: .starred, flag: false) } let insertExpectation = expectation(description: "Inserted Statuses") - container.database.insertStatuses(statuses) { + container.database.insertStatuses(statuses) { error in + XCTAssertNil(error) insertExpectation.fulfill() } @@ -266,7 +333,17 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { waitForExpectations(timeout: 2) - XCTAssertEqual(container.database.selectPendingCount(), 0) + let selectPendingCountExpectation = expectation(description: "Did Select Pending Count") + container.database.selectPendingCount { result in + do { + let statusCount = try result.get() + XCTAssertEqual(statusCount, 0) + selectPendingCountExpectation.fulfill() + } catch { + XCTFail("Error unwrapping database result: \(error)") + } + } + waitForExpectations(timeout: 2) } func testSendUnstarredFailure() { @@ -274,7 +351,8 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { let statuses = articleIds.map { SyncStatus(articleID: $0, key: .starred, flag: false) } let insertExpectation = expectation(description: "Inserted Statuses") - container.database.insertStatuses(statuses) { + container.database.insertStatuses(statuses) { error in + XCTAssertNil(error) insertExpectation.fulfill() } @@ -298,7 +376,17 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { waitForExpectations(timeout: 2) - XCTAssertEqual(container.database.selectPendingCount(), statuses.count) + let selectPendingCountExpectation = expectation(description: "Did Select Pending Count") + container.database.selectPendingCount { result in + do { + let expectedCount = try result.get() + XCTAssertEqual(expectedCount, statuses.count) + selectPendingCountExpectation.fulfill() + } catch { + XCTFail("Error unwrapping database result: \(error)") + } + } + waitForExpectations(timeout: 2) } func testSendAllSuccess() { @@ -313,7 +401,8 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { } let insertExpectation = expectation(description: "Inserted Statuses") - container.database.insertStatuses(statuses) { + container.database.insertStatuses(statuses) { error in + XCTAssertNil(error) insertExpectation.fulfill() } @@ -346,7 +435,18 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { OperationQueue.main.addOperation(send) waitForExpectations(timeout: 2) - XCTAssertEqual(container.database.selectPendingCount(), 0) + + let selectPendingCountExpectation = expectation(description: "Did Select Pending Count") + container.database.selectPendingCount { result in + do { + let statusCount = try result.get() + XCTAssertEqual(statusCount, 0) + selectPendingCountExpectation.fulfill() + } catch { + XCTFail("Error unwrapping database result: \(error)") + } + } + waitForExpectations(timeout: 2) } func testSendAllFailure() { @@ -361,7 +461,8 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { } let insertExpectation = expectation(description: "Inserted Statuses") - container.database.insertStatuses(statuses) { + container.database.insertStatuses(statuses) { error in + XCTAssertNil(error) insertExpectation.fulfill() } @@ -396,6 +497,16 @@ class FeedlySendArticleStatusesOperationTests: XCTestCase { waitForExpectations(timeout: 2) - XCTAssertEqual(container.database.selectPendingCount(), statuses.count) + let selectPendingCountExpectation = expectation(description: "Did Select Pending Count") + container.database.selectPendingCount { result in + do { + let statusCount = try result.get() + XCTAssertEqual(statusCount, statuses.count) + selectPendingCountExpectation.fulfill() + } catch { + XCTFail("Error unwrapping database result: \(error)") + } + } + waitForExpectations(timeout: 2) } } diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlySetStarredArticlesOperationTests.swift b/Frameworks/Account/AccountTests/Feedly/FeedlySetStarredArticlesOperationTests.swift index e86ba2de3..cdc0be0ad 100644 --- a/Frameworks/Account/AccountTests/Feedly/FeedlySetStarredArticlesOperationTests.swift +++ b/Frameworks/Account/AccountTests/Feedly/FeedlySetStarredArticlesOperationTests.swift @@ -49,10 +49,15 @@ class FeedlySetStarredArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetch Article Ids") - account.fetchStarredArticleIDs { accountArticlesIDs in - XCTAssertTrue(accountArticlesIDs.isEmpty) - XCTAssertEqual(accountArticlesIDs, testIds) - fetchIdsExpectation.fulfill() + account.fetchStarredArticleIDs { accountArticlesIDsResult in + do { + let accountArticlesIDs = try accountArticlesIDsResult.get() + XCTAssertTrue(accountArticlesIDs.isEmpty) + XCTAssertEqual(accountArticlesIDs, testIds) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking articles IDs: \(error)") + } } waitForExpectations(timeout: 2) } @@ -73,9 +78,14 @@ class FeedlySetStarredArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetch Article Ids") - account.fetchStarredArticleIDs { accountArticlesIDs in - XCTAssertEqual(accountArticlesIDs.count, testIds.count) - fetchIdsExpectation.fulfill() + account.fetchStarredArticleIDs { accountArticlesIDsResult in + do { + let accountArticlesIDs = try accountArticlesIDsResult.get() + XCTAssertEqual(accountArticlesIDs.count, testIds.count) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking articles IDs: \(error)") + } } waitForExpectations(timeout: 2) } @@ -96,9 +106,14 @@ class FeedlySetStarredArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetch Article Ids") - account.fetchStarredArticleIDs { accountArticlesIDs in - XCTAssertEqual(accountArticlesIDs.count, testIds.count) - fetchIdsExpectation.fulfill() + account.fetchStarredArticleIDs { accountArticlesIDsResult in + do { + let accountArticlesIDs = try accountArticlesIDsResult.get() + XCTAssertEqual(accountArticlesIDs.count, testIds.count) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking articles IDs: \(error)") + } } waitForExpectations(timeout: 2) } @@ -134,9 +149,14 @@ class FeedlySetStarredArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetch Article Ids") - account.fetchStarredArticleIDs { remainingAccountArticlesIDs in - XCTAssertEqual(remainingAccountArticlesIDs, remainingStarredIds) - fetchIdsExpectation.fulfill() + account.fetchStarredArticleIDs { remainingAccountArticlesIDsResult in + do { + let remainingAccountArticlesIDs = try remainingAccountArticlesIDsResult.get() + XCTAssertEqual(remainingAccountArticlesIDs, remainingStarredIds) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking articles IDs: \(error)") + } } waitForExpectations(timeout: 2) } @@ -172,9 +192,14 @@ class FeedlySetStarredArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetch Article Ids") - account.fetchStarredArticleIDs { remainingAccountArticlesIDs in - XCTAssertEqual(remainingAccountArticlesIDs, remainingStarredIds) - fetchIdsExpectation.fulfill() + account.fetchStarredArticleIDs { remainingAccountArticlesIDsResult in + do { + let remainingAccountArticlesIDs = try remainingAccountArticlesIDsResult.get() + XCTAssertEqual(remainingAccountArticlesIDs, remainingStarredIds) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking articles IDs: \(error)") + } } waitForExpectations(timeout: 2) } @@ -221,15 +246,21 @@ class FeedlySetStarredArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetch Article Ids") - account.fetchStarredArticleIDs { accountArticlesIDs in - XCTAssertEqual(accountArticlesIDs, remainingStarredIds) - - let idsOfStarredArticles = Set(self.account - .fetchArticles(.articleIDs(remainingStarredIds)) - .filter { $0.status.boolStatus(forKey: .starred) == true } - .map { $0.articleID }) - - XCTAssertEqual(idsOfStarredArticles, remainingStarredIds) + account.fetchStarredArticleIDs { accountArticlesIDsResult in + do { + let accountArticlesIDs = try accountArticlesIDsResult.get() + XCTAssertEqual(accountArticlesIDs, remainingStarredIds) + + let idsOfStarredArticles = Set(try self.account + .fetchArticles(.articleIDs(remainingStarredIds)) + .filter { $0.status.boolStatus(forKey: .starred) == true } + .map { $0.articleID }) + + XCTAssertEqual(idsOfStarredArticles, remainingStarredIds) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking articles IDs: \(error)") + } } waitForExpectations(timeout: 2) } @@ -274,15 +305,21 @@ class FeedlySetStarredArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetch Article Ids") - account.fetchStarredArticleIDs { accountArticlesIDs in - XCTAssertEqual(accountArticlesIDs, remainingStarredIds) - - let idsOfStarredArticles = Set(self.account - .fetchArticles(.articleIDs(remainingStarredIds)) - .filter { $0.status.boolStatus(forKey: .starred) == true } - .map { $0.articleID }) - - XCTAssertEqual(idsOfStarredArticles, remainingStarredIds) + account.fetchStarredArticleIDs { accountArticlesIDsResult in + do { + let accountArticlesIDs = try accountArticlesIDsResult.get() + XCTAssertEqual(accountArticlesIDs, remainingStarredIds) + + let idsOfStarredArticles = Set(try self.account + .fetchArticles(.articleIDs(remainingStarredIds)) + .filter { $0.status.boolStatus(forKey: .starred) == true } + .map { $0.articleID }) + + XCTAssertEqual(idsOfStarredArticles, remainingStarredIds) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking articles IDs: \(error)") + } } waitForExpectations(timeout: 2) } @@ -321,16 +358,21 @@ class FeedlySetStarredArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetch Article Ids") - account.fetchStarredArticleIDs { accountArticlesIDs in - XCTAssertEqual(accountArticlesIDs, remainingStarredIds) - - let idsOfStarredArticles = Set(self.account - .fetchArticles(.articleIDs(remainingStarredIds)) - .filter { $0.status.boolStatus(forKey: .starred) == true } - .map { $0.articleID }) - - XCTAssertEqual(idsOfStarredArticles, remainingStarredIds) - fetchIdsExpectation.fulfill() + account.fetchStarredArticleIDs { accountArticlesIDsResult in + do { + let accountArticlesIDs = try accountArticlesIDsResult.get() + XCTAssertEqual(accountArticlesIDs, remainingStarredIds) + + let idsOfStarredArticles = Set(try self.account + .fetchArticles(.articleIDs(remainingStarredIds)) + .filter { $0.status.boolStatus(forKey: .starred) == true } + .map { $0.articleID }) + + XCTAssertEqual(idsOfStarredArticles, remainingStarredIds) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking articles IDs: \(error)") + } } waitForExpectations(timeout: 2) } @@ -368,16 +410,21 @@ class FeedlySetStarredArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetch Article Ids") - account.fetchStarredArticleIDs { accountArticlesIDs in - XCTAssertEqual(accountArticlesIDs, remainingStarredIds) - - let idsOfStarredArticles = Set(self.account - .fetchArticles(.articleIDs(remainingStarredIds)) - .filter { $0.status.boolStatus(forKey: .starred) == true } - .map { $0.articleID }) - - XCTAssertEqual(idsOfStarredArticles, remainingStarredIds) - fetchIdsExpectation.fulfill() + account.fetchStarredArticleIDs { accountArticlesIDsResult in + do { + let accountArticlesIDs = try accountArticlesIDsResult.get() + XCTAssertEqual(accountArticlesIDs, remainingStarredIds) + + let idsOfStarredArticles = Set(try self.account + .fetchArticles(.articleIDs(remainingStarredIds)) + .filter { $0.status.boolStatus(forKey: .starred) == true } + .map { $0.articleID }) + + XCTAssertEqual(idsOfStarredArticles, remainingStarredIds) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking articles IDs: \(error)") + } } waitForExpectations(timeout: 2) } @@ -418,19 +465,24 @@ class FeedlySetStarredArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetch Article Ids") - account.fetchStarredArticleIDs { accountArticlesIDs in - XCTAssertEqual(accountArticlesIDs, remainingStarredIds) - - let someTestItems = Set(someItemsAndFeeds.flatMap { $0.value }) - let someRemainingStarredIdsOfIngestedArticles = Set(someTestItems.compactMap { $0.syncServiceID }) - let idsOfStarredArticles = Set(self.account - .fetchArticles(.articleIDs(someRemainingStarredIdsOfIngestedArticles)) - .filter { $0.status.boolStatus(forKey: .starred) == true } - .map { $0.articleID }) - - XCTAssertEqual(idsOfStarredArticles, someRemainingStarredIdsOfIngestedArticles) - - fetchIdsExpectation.fulfill() + account.fetchStarredArticleIDs { accountArticlesIDsResult in + do { + let accountArticlesIDs = try accountArticlesIDsResult.get() + XCTAssertEqual(accountArticlesIDs, remainingStarredIds) + + let someTestItems = Set(someItemsAndFeeds.flatMap { $0.value }) + let someRemainingStarredIdsOfIngestedArticles = Set(someTestItems.compactMap { $0.syncServiceID }) + let idsOfStarredArticles = Set(try self.account + .fetchArticles(.articleIDs(someRemainingStarredIdsOfIngestedArticles)) + .filter { $0.status.boolStatus(forKey: .starred) == true } + .map { $0.articleID }) + + XCTAssertEqual(idsOfStarredArticles, someRemainingStarredIdsOfIngestedArticles) + + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking articles IDs: \(error)") + } } waitForExpectations(timeout: 2) } diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlySetUnreadArticlesOperationTests.swift b/Frameworks/Account/AccountTests/Feedly/FeedlySetUnreadArticlesOperationTests.swift index e7e936e85..4126d6003 100644 --- a/Frameworks/Account/AccountTests/Feedly/FeedlySetUnreadArticlesOperationTests.swift +++ b/Frameworks/Account/AccountTests/Feedly/FeedlySetUnreadArticlesOperationTests.swift @@ -49,10 +49,15 @@ class FeedlySetUnreadArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetched Articles Ids") - account.fetchUnreadArticleIDs { accountArticlesIDs in - XCTAssertTrue(accountArticlesIDs.isEmpty) - XCTAssertEqual(accountArticlesIDs.count, testIds.count) - fetchIdsExpectation.fulfill() + account.fetchUnreadArticleIDs { accountArticlesIDsResult in + do { + let accountArticlesIDs = try accountArticlesIDsResult.get() + XCTAssertTrue(accountArticlesIDs.isEmpty) + XCTAssertEqual(accountArticlesIDs.count, testIds.count) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking account articles IDs result: \(error)") + } } waitForExpectations(timeout: 2) @@ -74,9 +79,14 @@ class FeedlySetUnreadArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetched Articles Ids") - account.fetchUnreadArticleIDs { accountArticlesIDs in - XCTAssertEqual(accountArticlesIDs.count, testIds.count) - fetchIdsExpectation.fulfill() + account.fetchUnreadArticleIDs { accountArticlesIDsResult in + do { + let accountArticlesIDs = try accountArticlesIDsResult.get() + XCTAssertEqual(accountArticlesIDs.count, testIds.count) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking account articles IDs result: \(error)") + } } waitForExpectations(timeout: 2) } @@ -97,9 +107,14 @@ class FeedlySetUnreadArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetched Articles Ids") - account.fetchUnreadArticleIDs { accountArticlesIDs in - XCTAssertEqual(accountArticlesIDs.count, testIds.count) - fetchIdsExpectation.fulfill() + account.fetchUnreadArticleIDs { accountArticlesIDsResult in + do { + let accountArticlesIDs = try accountArticlesIDsResult.get() + XCTAssertEqual(accountArticlesIDs.count, testIds.count) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking account articles IDs result: \(error)") + } } waitForExpectations(timeout: 2) } @@ -135,9 +150,14 @@ class FeedlySetUnreadArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetched Articles Ids") - account.fetchUnreadArticleIDs { remainingAccountArticlesIDs in - XCTAssertEqual(remainingAccountArticlesIDs, remainingUnreadIds) - fetchIdsExpectation.fulfill() + account.fetchUnreadArticleIDs { remainingAccountArticlesIDsResult in + do { + let remainingAccountArticlesIDs = try remainingAccountArticlesIDsResult.get() + XCTAssertEqual(remainingAccountArticlesIDs, remainingUnreadIds) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking account articles IDs result: \(error)") + } } waitForExpectations(timeout: 2) } @@ -173,9 +193,14 @@ class FeedlySetUnreadArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetched Articles Ids") - account.fetchUnreadArticleIDs { remainingAccountArticlesIDs in - XCTAssertEqual(remainingAccountArticlesIDs, remainingUnreadIds) - fetchIdsExpectation.fulfill() + account.fetchUnreadArticleIDs { remainingAccountArticlesIDsResult in + do { + let remainingAccountArticlesIDs = try remainingAccountArticlesIDsResult.get() + XCTAssertEqual(remainingAccountArticlesIDs, remainingUnreadIds) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking account articles IDs result: \(error)") + } } waitForExpectations(timeout: 2) } @@ -222,15 +247,20 @@ class FeedlySetUnreadArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetched Articles Ids") - account.fetchUnreadArticleIDs { accountArticlesIDs in - XCTAssertEqual(accountArticlesIDs, remainingUnreadIds) - let idsOfUnreadArticles = Set(self.account - .fetchArticles(.articleIDs(remainingUnreadIds)) - .filter { $0.status.boolStatus(forKey: .read) == false } - .map { $0.articleID }) - - XCTAssertEqual(idsOfUnreadArticles, remainingUnreadIds) - fetchIdsExpectation.fulfill() + account.fetchUnreadArticleIDs { accountArticlesIDsResult in + do { + let accountArticlesIDs = try accountArticlesIDsResult.get() + XCTAssertEqual(accountArticlesIDs, remainingUnreadIds) + let idsOfUnreadArticles = Set(try self.account + .fetchArticles(.articleIDs(remainingUnreadIds)) + .filter { $0.status.boolStatus(forKey: .read) == false } + .map { $0.articleID }) + + XCTAssertEqual(idsOfUnreadArticles, remainingUnreadIds) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking account articles IDs result: \(error)") + } } waitForExpectations(timeout: 2) } @@ -275,16 +305,21 @@ class FeedlySetUnreadArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetched Articles Ids") - account.fetchUnreadArticleIDs { accountArticlesIDs in - XCTAssertEqual(accountArticlesIDs, remainingUnreadIds) - - let idsOfUnreadArticles = Set(self.account - .fetchArticles(.articleIDs(remainingUnreadIds)) - .filter { $0.status.boolStatus(forKey: .read) == false } - .map { $0.articleID }) - - XCTAssertEqual(idsOfUnreadArticles, remainingUnreadIds) - fetchIdsExpectation.fulfill() + account.fetchUnreadArticleIDs { accountArticlesIDsResult in + do { + let accountArticlesIDs = try accountArticlesIDsResult.get() + XCTAssertEqual(accountArticlesIDs, remainingUnreadIds) + + let idsOfUnreadArticles = Set(try self.account + .fetchArticles(.articleIDs(remainingUnreadIds)) + .filter { $0.status.boolStatus(forKey: .read) == false } + .map { $0.articleID }) + + XCTAssertEqual(idsOfUnreadArticles, remainingUnreadIds) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking account articles IDs result: \(error)") + } } } @@ -322,16 +357,21 @@ class FeedlySetUnreadArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetched Articles Ids") - account.fetchUnreadArticleIDs { accountArticlesIDs in - XCTAssertEqual(accountArticlesIDs, remainingUnreadIds) - - let idsOfUnreadArticles = Set(self.account - .fetchArticles(.articleIDs(remainingUnreadIds)) - .filter { $0.status.boolStatus(forKey: .read) == false } - .map { $0.articleID }) - - XCTAssertEqual(idsOfUnreadArticles, remainingUnreadIds) - fetchIdsExpectation.fulfill() + account.fetchUnreadArticleIDs { accountArticlesIDsResult in + do { + let accountArticlesIDs = try accountArticlesIDsResult.get() + XCTAssertEqual(accountArticlesIDs, remainingUnreadIds) + + let idsOfUnreadArticles = Set(try self.account + .fetchArticles(.articleIDs(remainingUnreadIds)) + .filter { $0.status.boolStatus(forKey: .read) == false } + .map { $0.articleID }) + + XCTAssertEqual(idsOfUnreadArticles, remainingUnreadIds) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking account articles IDs result: \(error)") + } } waitForExpectations(timeout: 2) } @@ -369,16 +409,21 @@ class FeedlySetUnreadArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetched Articles Ids") - account.fetchUnreadArticleIDs { accountArticlesIDs in - XCTAssertEqual(accountArticlesIDs, remainingUnreadIds) - - let idsOfUnreadArticles = Set(self.account - .fetchArticles(.articleIDs(remainingUnreadIds)) - .filter { $0.status.boolStatus(forKey: .read) == false } - .map { $0.articleID }) - - XCTAssertEqual(idsOfUnreadArticles, remainingUnreadIds) - fetchIdsExpectation.fulfill() + account.fetchUnreadArticleIDs { accountArticlesIDsResult in + do { + let accountArticlesIDs = try accountArticlesIDsResult.get() + XCTAssertEqual(accountArticlesIDs, remainingUnreadIds) + + let idsOfUnreadArticles = Set(try self.account + .fetchArticles(.articleIDs(remainingUnreadIds)) + .filter { $0.status.boolStatus(forKey: .read) == false } + .map { $0.articleID }) + + XCTAssertEqual(idsOfUnreadArticles, remainingUnreadIds) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking account articles IDs result: \(error)") + } } } @@ -418,18 +463,23 @@ class FeedlySetUnreadArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetched Articles Ids") - account.fetchUnreadArticleIDs { accountArticlesIDs in - XCTAssertEqual(accountArticlesIDs, remainingUnreadIds) - - let someTestItems = Set(someItemsAndFeeds.flatMap { $0.value }) - let someRemainingUnreadIdsOfIngestedArticles = Set(someTestItems.compactMap { $0.syncServiceID }) - let idsOfUnreadArticles = Set(self.account - .fetchArticles(.articleIDs(someRemainingUnreadIdsOfIngestedArticles)) - .filter { $0.status.boolStatus(forKey: .read) == false } - .map { $0.articleID }) - - XCTAssertEqual(idsOfUnreadArticles, someRemainingUnreadIdsOfIngestedArticles) - fetchIdsExpectation.fulfill() + account.fetchUnreadArticleIDs { accountArticlesIDsResult in + do { + let accountArticlesIDs = try accountArticlesIDsResult.get() + XCTAssertEqual(accountArticlesIDs, remainingUnreadIds) + + let someTestItems = Set(someItemsAndFeeds.flatMap { $0.value }) + let someRemainingUnreadIdsOfIngestedArticles = Set(someTestItems.compactMap { $0.syncServiceID }) + let idsOfUnreadArticles = Set(try self.account + .fetchArticles(.articleIDs(someRemainingUnreadIdsOfIngestedArticles)) + .filter { $0.status.boolStatus(forKey: .read) == false } + .map { $0.articleID }) + + XCTAssertEqual(idsOfUnreadArticles, someRemainingUnreadIdsOfIngestedArticles) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking account articles IDs result: \(error)") + } } } } diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlySyncAllOperationTests.swift b/Frameworks/Account/AccountTests/Feedly/FeedlySyncAllOperationTests.swift index 914c25e8a..403250314 100644 --- a/Frameworks/Account/AccountTests/Feedly/FeedlySyncAllOperationTests.swift +++ b/Frameworks/Account/AccountTests/Feedly/FeedlySyncAllOperationTests.swift @@ -114,18 +114,18 @@ class FeedlySyncAllOperationTests: XCTestCase { return caller }() - func testSyncing() { + func testSyncing() throws { performInitialSync() - verifyInitialSync() + try verifyInitialSync() performChangeStatuses() - verifyChangeStatuses() + try verifyChangeStatuses() performChangeStatusesAgain() - verifyChangeStatusesAgain() + try verifyChangeStatusesAgain() performAddFeedsAndFolders() - verifyAddFeedsAndFolders() + try verifyAddFeedsAndFolders() } // MARK: 1 - Initial Sync @@ -166,15 +166,15 @@ class FeedlySyncAllOperationTests: XCTestCase { loadMockData(inSubdirectoryNamed: "feedly-1-initial") } - func verifyInitialSync() { + func verifyInitialSync() throws { let subdirectory = "feedly-1-initial" support.checkFoldersAndFeeds(in: account, againstCollectionsAndFeedsInJSONNamed: "collections", subdirectory: subdirectory) - support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "global.all", subdirectory: subdirectory) - support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "global.all@MTZkOTdkZWQ1NzM6NTE2OjUzYjgyNmEy", subdirectory: subdirectory) + try support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "global.all", subdirectory: subdirectory) + try support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "global.all@MTZkOTdkZWQ1NzM6NTE2OjUzYjgyNmEy", subdirectory: subdirectory) support.checkUnreadStatuses(in: account, againstIdsInStreamInJSONNamed: "unreadIds", subdirectory: subdirectory, testCase: self) support.checkUnreadStatuses(in: account, againstIdsInStreamInJSONNamed: "unreadIds@MTZkOTRhOTNhZTQ6MzExOjUzYjgyNmEy", subdirectory: subdirectory, testCase: self) support.checkStarredStatuses(in: account, againstItemsInStreamInJSONNamed: "starred", subdirectory: subdirectory, testCase: self) - support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "starred", subdirectory: subdirectory) + try support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "starred", subdirectory: subdirectory) } // MARK: 2 - Change Statuses @@ -183,14 +183,14 @@ class FeedlySyncAllOperationTests: XCTestCase { loadMockData(inSubdirectoryNamed: "feedly-2-changestatuses") } - func verifyChangeStatuses() { + func verifyChangeStatuses() throws { let subdirectory = "feedly-2-changestatuses" support.checkFoldersAndFeeds(in: account, againstCollectionsAndFeedsInJSONNamed: "collections", subdirectory: subdirectory) - support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "global.all", subdirectory: subdirectory) + try support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "global.all", subdirectory: subdirectory) support.checkUnreadStatuses(in: account, againstIdsInStreamInJSONNamed: "unreadIds", subdirectory: subdirectory, testCase: self) support.checkUnreadStatuses(in: account, againstIdsInStreamInJSONNamed: "unreadIds@MTZkOTJkNjIwM2Q6MTEzYjpkNDUwNjA3MQ==", subdirectory: subdirectory, testCase: self) support.checkStarredStatuses(in: account, againstItemsInStreamInJSONNamed: "starred", subdirectory: subdirectory, testCase: self) - support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "starred", subdirectory: subdirectory) + try support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "starred", subdirectory: subdirectory) } // MARK: 3 - Change Statuses Again @@ -199,14 +199,14 @@ class FeedlySyncAllOperationTests: XCTestCase { loadMockData(inSubdirectoryNamed: "feedly-3-changestatusesagain") } - func verifyChangeStatusesAgain() { + func verifyChangeStatusesAgain() throws { let subdirectory = "feedly-3-changestatusesagain" support.checkFoldersAndFeeds(in: account, againstCollectionsAndFeedsInJSONNamed: "collections", subdirectory: subdirectory) - support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "global.all", subdirectory: subdirectory) + try support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "global.all", subdirectory: subdirectory) support.checkUnreadStatuses(in: account, againstIdsInStreamInJSONNamed: "unreadIds", subdirectory: subdirectory, testCase: self) support.checkUnreadStatuses(in: account, againstIdsInStreamInJSONNamed: "unreadIds@MTZkOGRlMjVmM2M6M2YyOmQ0NTA2MDcx", subdirectory: subdirectory, testCase: self) support.checkStarredStatuses(in: account, againstItemsInStreamInJSONNamed: "starred", subdirectory: subdirectory, testCase: self) - support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "starred", subdirectory: subdirectory) + try support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "starred", subdirectory: subdirectory) } // MARK: 4 - Add Feeds and Folders @@ -215,14 +215,14 @@ class FeedlySyncAllOperationTests: XCTestCase { loadMockData(inSubdirectoryNamed: "feedly-4-addfeedsandfolders") } - func verifyAddFeedsAndFolders() { + func verifyAddFeedsAndFolders() throws { let subdirectory = "feedly-4-addfeedsandfolders" support.checkFoldersAndFeeds(in: account, againstCollectionsAndFeedsInJSONNamed: "collections", subdirectory: subdirectory) - support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "global.all", subdirectory: subdirectory) + try support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "global.all", subdirectory: subdirectory) support.checkUnreadStatuses(in: account, againstIdsInStreamInJSONNamed: "unreadIds", subdirectory: subdirectory, testCase: self) support.checkUnreadStatuses(in: account, againstIdsInStreamInJSONNamed: "unreadIds@MTZkOTE3YTRlMzQ6YWZjOmQ0NTA2MDcx", subdirectory: subdirectory, testCase: self) support.checkStarredStatuses(in: account, againstItemsInStreamInJSONNamed: "starred", subdirectory: subdirectory, testCase: self) - support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "starred", subdirectory: subdirectory) + try support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "starred", subdirectory: subdirectory) } // MARK: 5 - Remove Feeds and Folders @@ -231,14 +231,14 @@ class FeedlySyncAllOperationTests: XCTestCase { loadMockData(inSubdirectoryNamed: "feedly-5-removefeedsandfolders") } - func verifyRemoveFeedsAndFolders() { + func verifyRemoveFeedsAndFolders() throws { let subdirectory = "feedly-5-removefeedsandfolders" support.checkFoldersAndFeeds(in: account, againstCollectionsAndFeedsInJSONNamed: "collections", subdirectory: subdirectory) - support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "global.all", subdirectory: subdirectory) + try support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "global.all", subdirectory: subdirectory) support.checkUnreadStatuses(in: account, againstIdsInStreamInJSONNamed: "unreadIds", subdirectory: subdirectory, testCase: self) support.checkUnreadStatuses(in: account, againstIdsInStreamInJSONNamed: "unreadIds@MTZkOGRlMjVmM2M6M2YxOmQ0NTA2MDcx", subdirectory: subdirectory, testCase: self) support.checkStarredStatuses(in: account, againstItemsInStreamInJSONNamed: "starred", subdirectory: subdirectory, testCase: self) - support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "starred", subdirectory: subdirectory) + try support.checkArticles(in: account, againstItemsInStreamInJSONNamed: "starred", subdirectory: subdirectory) } // MARK: Downloading Test Data diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlySyncStarredArticlesOperationTests.swift b/Frameworks/Account/AccountTests/Feedly/FeedlySyncStarredArticlesOperationTests.swift index 761dd8a01..40d0c39ff 100644 --- a/Frameworks/Account/AccountTests/Feedly/FeedlySyncStarredArticlesOperationTests.swift +++ b/Frameworks/Account/AccountTests/Feedly/FeedlySyncStarredArticlesOperationTests.swift @@ -56,21 +56,26 @@ class FeedlySyncStarredArticlesOperationTests: XCTestCase { let expectedArticleIds = Set(items.map { $0.id }) let fetchIdsExpectation = expectation(description: "Fetch Article Ids") - account.fetchStarredArticleIDs { starredArticleIds in - let missingIds = expectedArticleIds.subtracting(starredArticleIds) - XCTAssertTrue(missingIds.isEmpty, "These article ids were not marked as starred.") - - // Fetch articles directly because account.fetchArticles(.starred) fetches starred articles for feeds subscribed to. - let expectedArticles = self.account.fetchArticles(.articleIDs(expectedArticleIds)) - XCTAssertEqual(expectedArticles.count, expectedArticleIds.count, "Did not fetch all the articles.") - - let starredArticles = self.account.fetchArticles(.articleIDs(starredArticleIds)) - XCTAssertEqual(expectedArticleIds.count, expectedArticles.count) - let missingArticles = expectedArticles.subtracting(starredArticles) - XCTAssertTrue(missingArticles.isEmpty, "These articles should be starred and fetched.") - XCTAssertEqual(expectedArticles, starredArticles) - - fetchIdsExpectation.fulfill() + account.fetchStarredArticleIDs { starredArticleIdsResult in + do { + let starredArticleIds = try starredArticleIdsResult.get() + let missingIds = expectedArticleIds.subtracting(starredArticleIds) + XCTAssertTrue(missingIds.isEmpty, "These article ids were not marked as starred.") + + // Fetch articles directly because account.fetchArticles(.starred) fetches starred articles for feeds subscribed to. + let expectedArticles = try self.account.fetchArticles(.articleIDs(expectedArticleIds)) + XCTAssertEqual(expectedArticles.count, expectedArticleIds.count, "Did not fetch all the articles.") + + let starredArticles = try self.account.fetchArticles(.articleIDs(starredArticleIds)) + XCTAssertEqual(expectedArticleIds.count, expectedArticles.count) + let missingArticles = expectedArticles.subtracting(starredArticles) + XCTAssertTrue(missingArticles.isEmpty, "These articles should be starred and fetched.") + XCTAssertEqual(expectedArticles, starredArticles) + + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking starred article IDs: \(error)") + } } waitForExpectations(timeout: 2) } @@ -104,9 +109,14 @@ class FeedlySyncStarredArticlesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetch Article Ids") - account.fetchStarredArticleIDs { starredArticleIds in - XCTAssertTrue(starredArticleIds.isEmpty) - fetchIdsExpectation.fulfill() + account.fetchStarredArticleIDs { starredArticleIdsResult in + do { + let starredArticleIds = try starredArticleIdsResult.get() + XCTAssertTrue(starredArticleIds.isEmpty) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking starred article IDs: \(error)") + } } waitForExpectations(timeout: 2) } @@ -153,21 +163,26 @@ class FeedlySyncStarredArticlesOperationTests: XCTestCase { // Find articles inserted. let expectedArticleIds = Set(service.pages.values.map { $0.items }.flatMap { $0 }.map { $0.id }) let fetchIdsExpectation = expectation(description: "Fetch Article Ids") - account.fetchStarredArticleIDs { starredArticleIds in - let missingIds = expectedArticleIds.subtracting(starredArticleIds) - XCTAssertTrue(missingIds.isEmpty, "These article ids were not marked as starred.") - - // Fetch articles directly because account.fetchArticles(.starred) fetches starred articles for feeds subscribed to. - let expectedArticles = self.account.fetchArticles(.articleIDs(expectedArticleIds)) - XCTAssertEqual(expectedArticles.count, expectedArticleIds.count, "Did not fetch all the articles.") - - let starredArticles = self.account.fetchArticles(.articleIDs(starredArticleIds)) - XCTAssertEqual(expectedArticleIds.count, expectedArticles.count) - let missingArticles = expectedArticles.subtracting(starredArticles) - XCTAssertTrue(missingArticles.isEmpty, "These articles should be starred and fetched.") - XCTAssertEqual(expectedArticles, starredArticles) - - fetchIdsExpectation.fulfill() + account.fetchStarredArticleIDs { starredArticleIdsResult in + do { + let starredArticleIds = try starredArticleIdsResult.get() + let missingIds = expectedArticleIds.subtracting(starredArticleIds) + XCTAssertTrue(missingIds.isEmpty, "These article ids were not marked as starred.") + + // Fetch articles directly because account.fetchArticles(.starred) fetches starred articles for feeds subscribed to. + let expectedArticles = try self.account.fetchArticles(.articleIDs(expectedArticleIds)) + XCTAssertEqual(expectedArticles.count, expectedArticleIds.count, "Did not fetch all the articles.") + + let starredArticles = try self.account.fetchArticles(.articleIDs(starredArticleIds)) + XCTAssertEqual(expectedArticleIds.count, expectedArticles.count) + let missingArticles = expectedArticles.subtracting(starredArticles) + XCTAssertTrue(missingArticles.isEmpty, "These articles should be starred and fetched.") + XCTAssertEqual(expectedArticles, starredArticles) + + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking starred article IDs: \(error)") + } } waitForExpectations(timeout: 2) } diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlySyncStreamContentsOperationTests.swift b/Frameworks/Account/AccountTests/Feedly/FeedlySyncStreamContentsOperationTests.swift index 52d03bef4..6a3f83d91 100644 --- a/Frameworks/Account/AccountTests/Feedly/FeedlySyncStreamContentsOperationTests.swift +++ b/Frameworks/Account/AccountTests/Feedly/FeedlySyncStreamContentsOperationTests.swift @@ -26,7 +26,7 @@ class FeedlySyncStreamContentsOperationTests: XCTestCase { super.tearDown() } - func testIngestsOnePageSuccess() { + func testIngestsOnePageSuccess() throws { let service = TestGetStreamContentsService() let resource = FeedlyCategoryResourceId(id: "user/1234/category/5678") let newerThan: Date? = Date(timeIntervalSinceReferenceDate: 0) @@ -56,7 +56,7 @@ class FeedlySyncStreamContentsOperationTests: XCTestCase { waitForExpectations(timeout: 2) let expectedArticleIds = Set(items.map { $0.id }) - let expectedArticles = account.fetchArticles(.articleIDs(expectedArticleIds)) + let expectedArticles = try account.fetchArticles(.articleIDs(expectedArticleIds)) XCTAssertEqual(expectedArticles.count, expectedArticleIds.count, "Did not fetch all the articles.") } @@ -90,7 +90,7 @@ class FeedlySyncStreamContentsOperationTests: XCTestCase { waitForExpectations(timeout: 2) } - func testIngestsManyPagesSuccess() { + func testIngestsManyPagesSuccess() throws { let service = TestGetPagedStreamContentsService() let resource = FeedlyCategoryResourceId(id: "user/1234/category/5678") let newerThan: Date? = Date(timeIntervalSinceReferenceDate: 0) @@ -132,7 +132,7 @@ class FeedlySyncStreamContentsOperationTests: XCTestCase { // Find articles inserted. let articleIds = Set(service.pages.values.map { $0.items }.flatMap { $0 }.map { $0.id }) - let articles = account.fetchArticles(.articleIDs(articleIds)) + let articles = try account.fetchArticles(.articleIDs(articleIds)) XCTAssertEqual(articleIds.count, articles.count) } } diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlySyncUnreadStatusesOperationTests.swift b/Frameworks/Account/AccountTests/Feedly/FeedlySyncUnreadStatusesOperationTests.swift index fa4864a73..09a8ec57a 100644 --- a/Frameworks/Account/AccountTests/Feedly/FeedlySyncUnreadStatusesOperationTests.swift +++ b/Frameworks/Account/AccountTests/Feedly/FeedlySyncUnreadStatusesOperationTests.swift @@ -56,10 +56,15 @@ class FeedlySyncUnreadStatusesOperationTests: XCTestCase { let expectedArticleIds = Set(ids) let fetchIdsExpectation = expectation(description: "Fetch Article Ids") - account.fetchUnreadArticleIDs { unreadArticleIds in - let missingIds = expectedArticleIds.subtracting(unreadArticleIds) - XCTAssertTrue(missingIds.isEmpty, "These article ids were not marked as unread.") - fetchIdsExpectation.fulfill() + account.fetchUnreadArticleIDs { unreadArticleIdsResult in + do { + let unreadArticleIds = try unreadArticleIdsResult.get() + let missingIds = expectedArticleIds.subtracting(unreadArticleIds) + XCTAssertTrue(missingIds.isEmpty, "These article ids were not marked as unread.") + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking unread article IDs: \(error)") + } } waitForExpectations(timeout: 2) } @@ -93,9 +98,14 @@ class FeedlySyncUnreadStatusesOperationTests: XCTestCase { waitForExpectations(timeout: 2) let fetchIdsExpectation = expectation(description: "Fetch Article Ids") - account.fetchUnreadArticleIDs { unreadArticleIds in - XCTAssertTrue(unreadArticleIds.isEmpty) - fetchIdsExpectation.fulfill() + account.fetchUnreadArticleIDs { unreadArticleIdsResult in + do { + let unreadArticleIds = try unreadArticleIdsResult.get() + XCTAssertTrue(unreadArticleIds.isEmpty) + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking unread article IDs: \(error)") + } } waitForExpectations(timeout: 2) } @@ -142,10 +152,15 @@ class FeedlySyncUnreadStatusesOperationTests: XCTestCase { // Find statuses inserted. let expectedArticleIds = Set(service.pages.values.map { $0.ids }.flatMap { $0 }) let fetchIdsExpectation = expectation(description: "Fetch Article Ids") - account.fetchUnreadArticleIDs { unreadArticleIds in - let missingIds = expectedArticleIds.subtracting(unreadArticleIds) - XCTAssertTrue(missingIds.isEmpty, "These article ids were not marked as unread.") - fetchIdsExpectation.fulfill() + account.fetchUnreadArticleIDs { unreadArticleIdsResult in + do { + let unreadArticleIds = try unreadArticleIdsResult.get() + let missingIds = expectedArticleIds.subtracting(unreadArticleIds) + XCTAssertTrue(missingIds.isEmpty, "These article ids were not marked as unread.") + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error checking unread article IDs: \(error)") + } } waitForExpectations(timeout: 2) } diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlyTestSupport.swift b/Frameworks/Account/AccountTests/Feedly/FeedlyTestSupport.swift index 3e5ae30ff..46117de80 100644 --- a/Frameworks/Account/AccountTests/Feedly/FeedlyTestSupport.swift +++ b/Frameworks/Account/AccountTests/Feedly/FeedlyTestSupport.swift @@ -141,13 +141,13 @@ class FeedlyTestSupport { XCTAssertTrue(missingFeedIds.isEmpty, "Feeds with these ids were not found in the \"\(label)\" folder.") } - func checkArticles(in account: Account, againstItemsInStreamInJSONNamed name: String, subdirectory: String? = nil) { + func checkArticles(in account: Account, againstItemsInStreamInJSONNamed name: String, subdirectory: String? = nil) throws { let stream = testJSON(named: name, subdirectory: subdirectory) as! [String:Any] - checkArticles(in: account, againstItemsInStreamInJSONPayload: stream) + try checkArticles(in: account, againstItemsInStreamInJSONPayload: stream) } - func checkArticles(in account: Account, againstItemsInStreamInJSONPayload stream: [String: Any]) { - checkArticles(in: account, correspondToStreamItemsIn: stream) + func checkArticles(in account: Account, againstItemsInStreamInJSONPayload stream: [String: Any]) throws { + try checkArticles(in: account, correspondToStreamItemsIn: stream) } private struct ArticleItem { @@ -188,13 +188,13 @@ class FeedlyTestSupport { } /// Awkwardly titled to make it clear the JSON given is from a stream response. - func checkArticles(in testAccount: Account, correspondToStreamItemsIn stream: [String: Any]) { + func checkArticles(in testAccount: Account, correspondToStreamItemsIn stream: [String: Any]) throws { let items = stream["items"] as! [[String: Any]] let articleItems = items.map { ArticleItem(item: $0) } let itemIds = Set(articleItems.map { $0.id }) - let articles = testAccount.fetchArticles(.articleIDs(itemIds)) + let articles = try testAccount.fetchArticles(.articleIDs(itemIds)) let articleIds = Set(articles.map { $0.articleID }) let missing = itemIds.subtracting(articleIds) @@ -220,12 +220,17 @@ class FeedlyTestSupport { func checkUnreadStatuses(in testAccount: Account, correspondToIdsInJSONPayload streamIds: [String: Any], testCase: XCTestCase) { let ids = Set(streamIds["ids"] as! [String]) let fetchIdsExpectation = testCase.expectation(description: "Fetch Article Ids") - testAccount.fetchUnreadArticleIDs { articleIds in - // Unread statuses can be paged from Feedly. - // Instead of joining test data, the best we can do is - // make sure that these ids are marked as unread (a subset of the total). - XCTAssertTrue(ids.isSubset(of: articleIds), "Some articles in `ids` are not marked as unread.") - fetchIdsExpectation.fulfill() + testAccount.fetchUnreadArticleIDs { articleIdsResult in + do { + let articleIds = try articleIdsResult.get() + // Unread statuses can be paged from Feedly. + // Instead of joining test data, the best we can do is + // make sure that these ids are marked as unread (a subset of the total). + XCTAssertTrue(ids.isSubset(of: articleIds), "Some articles in `ids` are not marked as unread.") + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error unwrapping article IDs: \(error)") + } } testCase.wait(for: [fetchIdsExpectation], timeout: 2) } @@ -239,12 +244,17 @@ class FeedlyTestSupport { let items = stream["items"] as! [[String: Any]] let ids = Set(items.map { $0["id"] as! String }) let fetchIdsExpectation = testCase.expectation(description: "Fetch Article Ids") - testAccount.fetchStarredArticleIDs { articleIds in - // Starred articles can be paged from Feedly. - // Instead of joining test data, the best we can do is - // make sure that these articles are marked as starred (a subset of the total). - XCTAssertTrue(ids.isSubset(of: articleIds), "Some articles in `ids` are not marked as starred.") - fetchIdsExpectation.fulfill() + testAccount.fetchStarredArticleIDs { articleIdsResult in + do { + let articleIds = try articleIdsResult.get() + // Starred articles can be paged from Feedly. + // Instead of joining test data, the best we can do is + // make sure that these articles are marked as starred (a subset of the total). + XCTAssertTrue(ids.isSubset(of: articleIds), "Some articles in `ids` are not marked as starred.") + fetchIdsExpectation.fulfill() + } catch { + XCTFail("Error unwrapping article IDs: \(error)") + } } testCase.wait(for: [fetchIdsExpectation], timeout: 2) } diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlyUpdateAccountFeedsWithItemsOperationTests.swift b/Frameworks/Account/AccountTests/Feedly/FeedlyUpdateAccountFeedsWithItemsOperationTests.swift index 7e0b4d087..b828f86e5 100644 --- a/Frameworks/Account/AccountTests/Feedly/FeedlyUpdateAccountFeedsWithItemsOperationTests.swift +++ b/Frameworks/Account/AccountTests/Feedly/FeedlyUpdateAccountFeedsWithItemsOperationTests.swift @@ -32,7 +32,7 @@ class FeedlyUpdateAccountFeedsWithItemsOperationTests: XCTestCase { var parsedItemsKeyedByFeedId: [String: Set] } - func testUpdateAccountWithEmptyItems() { + func testUpdateAccountWithEmptyItems() throws { let testItems = support.makeParsedItemTestDataFor(numberOfFeeds: 0, numberOfItemsInFeeds: 0) let resource = FeedlyCategoryResourceId(id: "user/12345/category/6789") let provider = TestItemsByFeedProvider(providerName: resource.id, parsedItemsKeyedByFeedId: testItems) @@ -52,11 +52,11 @@ class FeedlyUpdateAccountFeedsWithItemsOperationTests: XCTestCase { let articleIds = Set(entries.compactMap { $0.syncServiceID }) XCTAssertEqual(articleIds.count, entries.count, "Not every item has a value for \(\ParsedItem.syncServiceID).") - let accountArticles = account.fetchArticles(.articleIDs(articleIds)) + let accountArticles = try account.fetchArticles(.articleIDs(articleIds)) XCTAssertTrue(accountArticles.isEmpty) } - func testUpdateAccountWithOneItem() { + func testUpdateAccountWithOneItem() throws { let testItems = support.makeParsedItemTestDataFor(numberOfFeeds: 1, numberOfItemsInFeeds: 1) let resource = FeedlyCategoryResourceId(id: "user/12345/category/6789") let provider = TestItemsByFeedProvider(providerName: resource.id, parsedItemsKeyedByFeedId: testItems) @@ -76,7 +76,7 @@ class FeedlyUpdateAccountFeedsWithItemsOperationTests: XCTestCase { let articleIds = Set(entries.compactMap { $0.syncServiceID }) XCTAssertEqual(articleIds.count, entries.count, "Not every item has a value for \(\ParsedItem.syncServiceID).") - let accountArticles = account.fetchArticles(.articleIDs(articleIds)) + let accountArticles = try account.fetchArticles(.articleIDs(articleIds)) XCTAssertTrue(accountArticles.count == entries.count) let accountArticleIds = Set(accountArticles.map { $0.articleID }) @@ -84,7 +84,7 @@ class FeedlyUpdateAccountFeedsWithItemsOperationTests: XCTestCase { XCTAssertTrue(missingIds.isEmpty) } - func testUpdateAccountWithManyItems() { + func testUpdateAccountWithManyItems() throws { let testItems = support.makeParsedItemTestDataFor(numberOfFeeds: 100, numberOfItemsInFeeds: 100) let resource = FeedlyCategoryResourceId(id: "user/12345/category/6789") let provider = TestItemsByFeedProvider(providerName: resource.id, parsedItemsKeyedByFeedId: testItems) @@ -104,7 +104,7 @@ class FeedlyUpdateAccountFeedsWithItemsOperationTests: XCTestCase { let articleIds = Set(entries.compactMap { $0.syncServiceID }) XCTAssertEqual(articleIds.count, entries.count, "Not every item has a value for \(\ParsedItem.syncServiceID).") - let accountArticles = account.fetchArticles(.articleIDs(articleIds)) + let accountArticles = try account.fetchArticles(.articleIDs(articleIds)) XCTAssertTrue(accountArticles.count == entries.count) let accountArticleIds = Set(accountArticles.map { $0.articleID }) @@ -112,7 +112,7 @@ class FeedlyUpdateAccountFeedsWithItemsOperationTests: XCTestCase { XCTAssertTrue(missingIds.isEmpty) } - func testCancelUpdateAccount() { + func testCancelUpdateAccount() throws { let testItems = support.makeParsedItemTestDataFor(numberOfFeeds: 1, numberOfItemsInFeeds: 1) let resource = FeedlyCategoryResourceId(id: "user/12345/category/6789") let provider = TestItemsByFeedProvider(providerName: resource.id, parsedItemsKeyedByFeedId: testItems) @@ -134,7 +134,7 @@ class FeedlyUpdateAccountFeedsWithItemsOperationTests: XCTestCase { let articleIds = Set(entries.compactMap { $0.syncServiceID }) XCTAssertEqual(articleIds.count, entries.count, "Not every item has a value for \(\ParsedItem.syncServiceID).") - let accountArticles = account.fetchArticles(.articleIDs(articleIds)) + let accountArticles = try account.fetchArticles(.articleIDs(articleIds)) XCTAssertTrue(accountArticles.isEmpty) } } diff --git a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift index 14d2d4e49..fb5cad188 100644 --- a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift @@ -1237,20 +1237,38 @@ private extension FeedbinAccountDelegate { return } - let feedbinUnreadArticleIDs = Set(articleIDs.map { String($0) } ) - account.fetchUnreadArticleIDs { articleIDsResult in - guard let currentUnreadArticleIDs = try? articleIDsResult.get() else { - return + database.selectPendingReadStatusArticleIDs() { result in + + func process(_ pendingArticleIDs: Set) { + + let feedbinUnreadArticleIDs = Set(articleIDs.map { String($0) } ) + let updatableFeedbinUnreadArticleIDs = feedbinUnreadArticleIDs.subtracting(pendingArticleIDs) + + account.fetchUnreadArticleIDs { articleIDsResult in + guard let currentUnreadArticleIDs = try? articleIDsResult.get() else { + return + } + + // Mark articles as unread + let deltaUnreadArticleIDs = updatableFeedbinUnreadArticleIDs.subtracting(currentUnreadArticleIDs) + account.markAsUnread(deltaUnreadArticleIDs) + + // Mark articles as read + let deltaReadArticleIDs = currentUnreadArticleIDs.subtracting(updatableFeedbinUnreadArticleIDs) + account.markAsRead(deltaReadArticleIDs) + } + } - - // Mark articles as unread - let deltaUnreadArticleIDs = feedbinUnreadArticleIDs.subtracting(currentUnreadArticleIDs) - account.markAsUnread(deltaUnreadArticleIDs) - - // Mark articles as read - let deltaReadArticleIDs = currentUnreadArticleIDs.subtracting(feedbinUnreadArticleIDs) - account.markAsRead(deltaReadArticleIDs) + + switch result { + case .success(let pendingArticleIDs): + process(pendingArticleIDs) + case .failure(let error): + os_log(.error, log: self.log, "Sync Article Read Status failed: %@.", error.localizedDescription) + } + } + } func syncArticleStarredState(account: Account, articleIDs: [Int]?) { @@ -1258,20 +1276,38 @@ private extension FeedbinAccountDelegate { return } - let feedbinStarredArticleIDs = Set(articleIDs.map { String($0) } ) - account.fetchStarredArticleIDs { articleIDsResult in - guard let currentStarredArticleIDs = try? articleIDsResult.get() else { - return + database.selectPendingStarredStatusArticleIDs() { result in + + func process(_ pendingArticleIDs: Set) { + + let feedbinStarredArticleIDs = Set(articleIDs.map { String($0) } ) + let updatableFeedbinUnreadArticleIDs = feedbinStarredArticleIDs.subtracting(pendingArticleIDs) + + account.fetchStarredArticleIDs { articleIDsResult in + guard let currentStarredArticleIDs = try? articleIDsResult.get() else { + return + } + + // Mark articles as starred + let deltaStarredArticleIDs = updatableFeedbinUnreadArticleIDs.subtracting(currentStarredArticleIDs) + account.markAsStarred(deltaStarredArticleIDs) + + // Mark articles as unstarred + let deltaUnstarredArticleIDs = currentStarredArticleIDs.subtracting(updatableFeedbinUnreadArticleIDs) + account.markAsUnstarred(deltaUnstarredArticleIDs) + } + + } + + switch result { + case .success(let pendingArticleIDs): + process(pendingArticleIDs) + case .failure(let error): + os_log(.error, log: self.log, "Sync Article Starred Status failed: %@.", error.localizedDescription) } - // Mark articles as starred - let deltaStarredArticleIDs = feedbinStarredArticleIDs.subtracting(currentStarredArticleIDs) - account.markAsStarred(deltaStarredArticleIDs) - - // Mark articles as unstarred - let deltaUnstarredArticleIDs = currentStarredArticleIDs.subtracting(feedbinStarredArticleIDs) - account.markAsUnstarred(deltaUnstarredArticleIDs) } + } func deleteTagging(for account: Account, with feed: WebFeed, from container: Container?, completion: @escaping (Result) -> Void) { diff --git a/Frameworks/Account/Feedly/Models/FeedlyEntryParser.swift b/Frameworks/Account/Feedly/Models/FeedlyEntryParser.swift index 5db8b2c0c..4fe7589f3 100644 --- a/Frameworks/Account/Feedly/Models/FeedlyEntryParser.swift +++ b/Frameworks/Account/Feedly/Models/FeedlyEntryParser.swift @@ -17,10 +17,10 @@ struct FeedlyEntryParser { return entry.id } - var feedUrl: String { + var feedUrl: String? { guard let id = entry.origin?.streamId else { assertionFailure() - return "" + return nil } return id } @@ -82,7 +82,11 @@ struct FeedlyEntryParser { return attachments.isEmpty ? nil : Set(attachments) } - var parsedItemRepresentation: ParsedItem { + var parsedItemRepresentation: ParsedItem? { + guard let feedUrl = feedUrl else { + return nil + } + return ParsedItem(syncServiceID: id, uniqueID: id, // This value seems to get ignored or replaced. feedURL: feedUrl, diff --git a/Frameworks/Account/Feedly/Models/FeedlyOrigin.swift b/Frameworks/Account/Feedly/Models/FeedlyOrigin.swift index 3ecb6528e..1de7c0434 100644 --- a/Frameworks/Account/Feedly/Models/FeedlyOrigin.swift +++ b/Frameworks/Account/Feedly/Models/FeedlyOrigin.swift @@ -10,6 +10,6 @@ import Foundation struct FeedlyOrigin: Decodable { var title: String? - var streamId: String - var htmlUrl: String + var streamId: String? + var htmlUrl: String? } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyGetStreamContentsOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyGetStreamContentsOperation.swift index 40bb6080b..db8c1a201 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyGetStreamContentsOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyGetStreamContentsOperation.swift @@ -50,7 +50,17 @@ final class FeedlyGetStreamContentsOperation: FeedlyOperation, FeedlyEntryProvid return entries } - let parsed = Set(entries.map { FeedlyEntryParser(entry: $0).parsedItemRepresentation }) + let parsed = Set(entries.compactMap { + FeedlyEntryParser(entry: $0).parsedItemRepresentation + }) + + if parsed.count != entries.count { + let entryIds = Set(entries.map { $0.id }) + let parsedIds = Set(parsed.map { $0.uniqueID }) + let difference = entryIds.subtracting(parsedIds) + os_log(.debug, log: log, "Dropping articles with ids: %{public}@.", difference) + } + storedParsedEntries = parsed return parsed diff --git a/Frameworks/Account/Feedly/Operations/FeedlyOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyOperation.swift index f1ea40369..bf5dae694 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyOperation.swift @@ -29,14 +29,17 @@ class FeedlyOperation: Operation { } } + override var isAsynchronous: Bool { + return true + } + func didFinish() { assert(Thread.isMainThread) assert(!isFinished, "Finished operation is attempting to finish again.") downloadProgress = nil - isExecutingOperation = false - isFinishedOperation = true + updateExecutingAndFinished(false, true) } func didFinish(_ error: Error) { @@ -58,9 +61,8 @@ class FeedlyOperation: Operation { override func start() { guard !isCancelled else { - isExecutingOperation = false - isFinishedOperation = true - + updateExecutingAndFinished(false, true) + if downloadProgress != nil { DispatchQueue.main.async { self.downloadProgress = nil @@ -69,8 +71,8 @@ class FeedlyOperation: Operation { return } - - isExecutingOperation = true + + updateExecutingAndFinished(true, false) DispatchQueue.main.async { self.main() } @@ -80,25 +82,31 @@ class FeedlyOperation: Operation { return isExecutingOperation } - private var isExecutingOperation = false { - willSet { - willChangeValue(for: \.isExecuting) - } - didSet { - didChangeValue(for: \.isExecuting) - } - } - override var isFinished: Bool { return isFinishedOperation } - - private var isFinishedOperation = false { - willSet { - willChangeValue(for: \.isFinished) + + private var isExecutingOperation = false + private var isFinishedOperation = false + + private func updateExecutingAndFinished(_ executing: Bool, _ finished: Bool) { + let isExecutingDidChange = executing != isExecutingOperation + let isFinishedDidChange = finished != isFinishedOperation + + if isFinishedDidChange { + willChangeValue(forKey: #keyPath(isFinished)) } - didSet { - didChangeValue(for: \.isFinished) + if isExecutingDidChange { + willChangeValue(forKey: #keyPath(isExecuting)) + } + isExecutingOperation = executing + isFinishedOperation = finished + + if isExecutingDidChange { + didChangeValue(forKey: #keyPath(isExecuting)) + } + if isFinishedDidChange { + didChangeValue(forKey: #keyPath(isFinished)) } } } diff --git a/Frameworks/Articles/Article.swift b/Frameworks/Articles/Article.swift index 85ebe8385..6e13633cc 100644 --- a/Frameworks/Articles/Article.swift +++ b/Frameworks/Articles/Article.swift @@ -23,13 +23,12 @@ public struct Article: Hashable { public let externalURL: String? public let summary: String? public let imageURL: String? - public let bannerImageURL: String? public let datePublished: Date? public let dateModified: Date? public let authors: Set? public let status: ArticleStatus - public init(accountID: String, articleID: String?, webFeedID: String, uniqueID: String, title: String?, contentHTML: String?, contentText: String?, url: String?, externalURL: String?, summary: String?, imageURL: String?, bannerImageURL: String?, datePublished: Date?, dateModified: Date?, authors: Set?, status: ArticleStatus) { + public init(accountID: String, articleID: String?, webFeedID: String, uniqueID: String, title: String?, contentHTML: String?, contentText: String?, url: String?, externalURL: String?, summary: String?, imageURL: String?, datePublished: Date?, dateModified: Date?, authors: Set?, status: ArticleStatus) { self.accountID = accountID self.webFeedID = webFeedID self.uniqueID = uniqueID @@ -40,7 +39,6 @@ public struct Article: Hashable { self.externalURL = externalURL self.summary = summary self.imageURL = imageURL - self.bannerImageURL = bannerImageURL self.datePublished = datePublished self.dateModified = dateModified self.authors = authors diff --git a/Frameworks/ArticlesDatabase/ArticlesDatabase.xcodeproj/project.pbxproj b/Frameworks/ArticlesDatabase/ArticlesDatabase.xcodeproj/project.pbxproj index f3241666f..a93a2be9d 100644 --- a/Frameworks/ArticlesDatabase/ArticlesDatabase.xcodeproj/project.pbxproj +++ b/Frameworks/ArticlesDatabase/ArticlesDatabase.xcodeproj/project.pbxproj @@ -11,7 +11,6 @@ 841D4D742106B59F00DD04E6 /* Articles.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 841D4D732106B59F00DD04E6 /* Articles.framework */; }; 84288A001F6A3C4400395871 /* DatabaseObject+Database.swift in Sources */ = {isa = PBXBuildFile; fileRef = 842889FF1F6A3C4400395871 /* DatabaseObject+Database.swift */; }; 84288A021F6A3D8000395871 /* RelatedObjectsMap+Database.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84288A011F6A3D8000395871 /* RelatedObjectsMap+Database.swift */; }; - 843577161F744FC800F460AE /* DatabaseArticle.swift in Sources */ = {isa = PBXBuildFile; fileRef = 843577151F744FC800F460AE /* DatabaseArticle.swift */; }; 843577221F749C6200F460AE /* ArticleChangesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 843577211F749C6200F460AE /* ArticleChangesTests.swift */; }; 843702C31F70D15D00B18807 /* ParsedArticle+Database.swift in Sources */ = {isa = PBXBuildFile; fileRef = 843702C21F70D15D00B18807 /* ParsedArticle+Database.swift */; }; 843CB9961F34174100EE6581 /* Author+Database.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84F20F901F1810DD00D8E682 /* Author+Database.swift */; }; @@ -115,7 +114,6 @@ 841D4D732106B59F00DD04E6 /* Articles.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; path = Articles.framework; sourceTree = BUILT_PRODUCTS_DIR; }; 842889FF1F6A3C4400395871 /* DatabaseObject+Database.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DatabaseObject+Database.swift"; sourceTree = ""; }; 84288A011F6A3D8000395871 /* RelatedObjectsMap+Database.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "RelatedObjectsMap+Database.swift"; sourceTree = ""; }; - 843577151F744FC800F460AE /* DatabaseArticle.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DatabaseArticle.swift; sourceTree = ""; }; 843577211F749C6200F460AE /* ArticleChangesTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ArticleChangesTests.swift; sourceTree = ""; }; 843702C21F70D15D00B18807 /* ParsedArticle+Database.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = "ParsedArticle+Database.swift"; path = "Extensions/ParsedArticle+Database.swift"; sourceTree = ""; }; 844BEE371F0AB3AA004AB7CD /* ArticlesDatabase.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = ArticlesDatabase.framework; sourceTree = BUILT_PRODUCTS_DIR; }; @@ -176,7 +174,6 @@ 845580661F0AEBCD003CCFA1 /* Constants.swift */, 84E156EB1F0AB80E00F8CC05 /* ArticlesTable.swift */, 8477ACBB2221E76F00DF7F37 /* SearchTable.swift */, - 843577151F744FC800F460AE /* DatabaseArticle.swift */, 84E156ED1F0AB81400F8CC05 /* StatusesTable.swift */, 84F20F8E1F180D8700D8E682 /* AuthorsTable.swift */, 8461462A1F0AC44100870CB3 /* Extensions */, @@ -350,14 +347,14 @@ TargetAttributes = { 844BEE361F0AB3AA004AB7CD = { CreatedOnToolsVersion = 8.3.2; - DevelopmentTeam = SHJK2V3AJG; + DevelopmentTeam = M8L2WTLA8W; LastSwiftMigration = 0830; - ProvisioningStyle = Automatic; + ProvisioningStyle = Manual; }; 844BEE3F1F0AB3AB004AB7CD = { CreatedOnToolsVersion = 8.3.2; - DevelopmentTeam = SHJK2V3AJG; - ProvisioningStyle = Automatic; + DevelopmentTeam = M8L2WTLA8W; + ProvisioningStyle = Manual; }; }; }; @@ -522,7 +519,6 @@ 84F20F8F1F180D8700D8E682 /* AuthorsTable.swift in Sources */, 84288A001F6A3C4400395871 /* DatabaseObject+Database.swift in Sources */, 8477ACBC2221E76F00DF7F37 /* SearchTable.swift in Sources */, - 843577161F744FC800F460AE /* DatabaseArticle.swift in Sources */, 843702C31F70D15D00B18807 /* ParsedArticle+Database.swift in Sources */, 84E156EC1F0AB80E00F8CC05 /* ArticlesTable.swift in Sources */, 84E156EE1F0AB81400F8CC05 /* StatusesTable.swift in Sources */, diff --git a/Frameworks/ArticlesDatabase/ArticlesTable.swift b/Frameworks/ArticlesDatabase/ArticlesTable.swift index 877b1b0b6..226df54ab 100644 --- a/Frameworks/ArticlesDatabase/ArticlesTable.swift +++ b/Frameworks/ArticlesDatabase/ArticlesTable.swift @@ -19,14 +19,14 @@ final class ArticlesTable: DatabaseTable { private let queue: DatabaseQueue private let statusesTable: StatusesTable private let authorsLookupTable: DatabaseLookupTable - private var databaseArticlesCache = [String: DatabaseArticle]() + private var articlesCache = [String: Article]() private lazy var searchTable: SearchTable = { return SearchTable(queue: queue, articlesTable: self) }() // TODO: update articleCutoffDate as time passes and based on user preferences. - private var articleCutoffDate = NSDate.rs_dateWithNumberOfDays(inThePast: 90)! + private let articleCutoffDate = Date().bySubtracting(days: 90) private typealias ArticlesFetchMethod = (FMDatabase) -> Set
@@ -212,6 +212,9 @@ final class ArticlesTable: DatabaseTable { self.callUpdateArticlesCompletionBlock(newArticles, updatedArticles, completion) //7 + self.addArticlesToCache(newArticles) + self.addArticlesToCache(updatedArticles) + // 8. Update search index. if let newArticles = newArticles { self.searchTable.indexNewArticles(newArticles, database) @@ -302,9 +305,9 @@ final class ArticlesTable: DatabaseTable { queue.runInDatabase { databaseResult in func makeDatabaseCalls(_ database: FMDatabase) { - let sql = "select distinct feedID, count(*) from articles natural join statuses where read=0 and userDeleted=0 and (starred=1 or (datePublished > ? or (datePublished is null and dateArrived > ?))) group by feedID;" + let sql = "select distinct feedID, count(*) from articles natural join statuses where read=0 and userDeleted=0 and (starred=1 or dateArrived>?) group by feedID;" - guard let resultSet = database.executeQuery(sql, withArgumentsIn: [cutoffDate, cutoffDate]) else { + guard let resultSet = database.executeQuery(sql, withArgumentsIn: [cutoffDate]) else { DispatchQueue.main.async { completion(.success(UnreadCountDictionary())) } @@ -449,7 +452,7 @@ final class ArticlesTable: DatabaseTable { func emptyCaches() { queue.runInDatabase { _ in - self.databaseArticlesCache = [String: DatabaseArticle]() + self.articlesCache = [String: Article]() } } @@ -527,86 +530,55 @@ private extension ArticlesTable { } func articlesWithResultSet(_ resultSet: FMResultSet, _ database: FMDatabase) -> Set
{ - // 1. Create DatabaseArticles without related objects. - // 2. Then fetch the related objects, given the set of articleIDs. - // 3. Then create set of Articles with DatabaseArticles and related objects and return it. + var cachedArticles = Set
() + var fetchedArticles = Set
() - // 1. Create databaseArticles (intermediate representations). + while resultSet.next() { - let databaseArticles = makeDatabaseArticles(with: resultSet) - if databaseArticles.isEmpty { - return Set
() - } - - let articleIDs = databaseArticles.articleIDs() - - // 2. Fetch related objects. - - let authorsMap = authorsLookupTable.fetchRelatedObjects(for: articleIDs, in: database) - - // 3. Create articles with related objects. - - let articles = databaseArticles.map { (databaseArticle) -> Article in - return articleWithDatabaseArticle(databaseArticle, authorsMap) - } - - return Set(articles) - } - - func articleWithDatabaseArticle(_ databaseArticle: DatabaseArticle, _ authorsMap: RelatedObjectsMap?) -> Article { - - let articleID = databaseArticle.articleID - let authors = authorsMap?.authors(for: articleID) - - return Article(databaseArticle: databaseArticle, accountID: accountID, authors: authors) - } - - func makeDatabaseArticles(with resultSet: FMResultSet) -> Set { - let articles = resultSet.mapToSet { (row) -> DatabaseArticle? in - - guard let articleID = row.string(forColumn: DatabaseKey.articleID) else { + guard let articleID = resultSet.string(forColumn: DatabaseKey.articleID) else { assertionFailure("Expected articleID.") - return nil + continue } - // Articles are removed from the cache when they’re updated. - // See saveUpdatedArticles. - if let databaseArticle = databaseArticlesCache[articleID] { - return databaseArticle + if let article = articlesCache[articleID] { + cachedArticles.insert(article) + continue } // The resultSet is a result of a JOIN query with the statuses table, // so we can get the statuses at the same time and avoid additional database lookups. guard let status = statusesTable.statusWithRow(resultSet, articleID: articleID) else { assertionFailure("Expected status.") - return nil - } - guard let webFeedID = row.string(forColumn: DatabaseKey.feedID) else { - assertionFailure("Expected feedID.") - return nil - } - guard let uniqueID = row.string(forColumn: DatabaseKey.uniqueID) else { - assertionFailure("Expected uniqueID.") - return nil + continue } - let title = row.string(forColumn: DatabaseKey.title) - let contentHTML = row.string(forColumn: DatabaseKey.contentHTML) - let contentText = row.string(forColumn: DatabaseKey.contentText) - let url = row.string(forColumn: DatabaseKey.url) - let externalURL = row.string(forColumn: DatabaseKey.externalURL) - let summary = row.string(forColumn: DatabaseKey.summary) - let imageURL = row.string(forColumn: DatabaseKey.imageURL) - let bannerImageURL = row.string(forColumn: DatabaseKey.bannerImageURL) - let datePublished = row.date(forColumn: DatabaseKey.datePublished) - let dateModified = row.date(forColumn: DatabaseKey.dateModified) + guard let article = Article(accountID: accountID, row: resultSet, status: status) else { + continue + } + fetchedArticles.insert(article) + } + resultSet.close() - let databaseArticle = DatabaseArticle(articleID: articleID, webFeedID: webFeedID, uniqueID: uniqueID, title: title, contentHTML: contentHTML, contentText: contentText, url: url, externalURL: externalURL, summary: summary, imageURL: imageURL, bannerImageURL: bannerImageURL, datePublished: datePublished, dateModified: dateModified, status: status) - databaseArticlesCache[articleID] = databaseArticle - return databaseArticle + if fetchedArticles.isEmpty { + return cachedArticles } - return articles + // Fetch authors for non-cached articles. (Articles from the cache already have authors.) + let fetchedArticleIDs = fetchedArticles.articleIDs() + let authorsMap = authorsLookupTable.fetchRelatedObjects(for: fetchedArticleIDs, in: database) + let articlesWithFetchedAuthors = fetchedArticles.map { (article) -> Article in + if let authors = authorsMap?.authors(for: article.articleID) { + return article.byAdding(authors) + } + return article + } + + // Add fetchedArticles to cache, now that they have attached authors. + for article in articlesWithFetchedAuthors { + articlesCache[article.articleID] = article + } + + return cachedArticles.union(articlesWithFetchedAuthors) } func fetchArticlesWithWhereClause(_ database: FMDatabase, whereClause: String, parameters: [AnyObject], withLimits: Bool) -> Set
{ @@ -615,8 +587,8 @@ private extension ArticlesTable { // * Must be either 1) starred or 2) dateArrived must be newer than cutoff date. if withLimits { - let sql = "select * from articles natural join statuses where \(whereClause) and userDeleted=0 and (starred=1 or (datePublished > ? or (datePublished is null and dateArrived > ?)));" - return articlesWithSQL(sql, parameters + [articleCutoffDate as AnyObject] + [articleCutoffDate as AnyObject], database) + let sql = "select * from articles natural join statuses where \(whereClause) and userDeleted=0 and (starred=1 or dateArrived>?);" + return articlesWithSQL(sql, parameters + [articleCutoffDate as AnyObject], database) } else { let sql = "select * from articles natural join statuses where \(whereClause);" @@ -630,8 +602,8 @@ private extension ArticlesTable { // * Must not be deleted. // * Must be either 1) starred or 2) dateArrived must be newer than cutoff date. - let sql = "select count(*) from articles natural join statuses where feedID=? and read=0 and userDeleted=0 and (starred=1 or (datePublished > ? or (datePublished is null and dateArrived > ?)));" - return numberWithSQLAndParameters(sql, [webFeedID, articleCutoffDate, articleCutoffDate], in: database) + let sql = "select count(*) from articles natural join statuses where feedID=? and read=0 and userDeleted=0 and (starred=1 or dateArrived>?);" + return numberWithSQLAndParameters(sql, [webFeedID, articleCutoffDate], in: database) } func fetchArticlesMatching(_ searchString: String, _ database: FMDatabase) -> Set
{ @@ -872,7 +844,6 @@ private extension ArticlesTable { func saveUpdatedArticles(_ updatedArticles: Set
, _ fetchedArticles: [String: Article], _ database: FMDatabase) { - removeArticlesFromDatabaseArticlesCache(updatedArticles) saveUpdatedRelatedObjects(updatedArticles, fetchedArticles, database) for updatedArticle in updatedArticles { @@ -897,10 +868,12 @@ private extension ArticlesTable { updateRowsWithDictionary(changesDictionary, whereKey: DatabaseKey.articleID, matches: updatedArticle.articleID, database: database) } - func removeArticlesFromDatabaseArticlesCache(_ updatedArticles: Set
) { - let articleIDs = updatedArticles.articleIDs() - for articleID in articleIDs { - databaseArticlesCache[articleID] = nil + func addArticlesToCache(_ articles: Set
?) { + guard let articles = articles else { + return + } + for article in articles { + articlesCache[article.articleID] = article } } @@ -912,9 +885,6 @@ private extension ArticlesTable { if article.status.starred { return false } - if let datePublished = article.datePublished { - return datePublished < articleCutoffDate - } return article.status.dateArrived < articleCutoffDate } diff --git a/Frameworks/ArticlesDatabase/DatabaseArticle.swift b/Frameworks/ArticlesDatabase/DatabaseArticle.swift deleted file mode 100644 index d029f7113..000000000 --- a/Frameworks/ArticlesDatabase/DatabaseArticle.swift +++ /dev/null @@ -1,44 +0,0 @@ -// -// DatabaseArticle.swift -// NetNewsWire -// -// Created by Brent Simmons on 9/21/17. -// Copyright © 2017 Ranchero Software. All rights reserved. -// - -import Foundation -import Articles - -// Intermediate representation of an Article. Doesn’t include related objects. -// Used by ArticlesTable as part of fetching articles. - -struct DatabaseArticle: Hashable { - - let articleID: String - let webFeedID: String - let uniqueID: String - let title: String? - let contentHTML: String? - let contentText: String? - let url: String? - let externalURL: String? - let summary: String? - let imageURL: String? - let bannerImageURL: String? - let datePublished: Date? - let dateModified: Date? - let status: ArticleStatus - - // MARK: - Hashable - - public func hash(into hasher: inout Hasher) { - hasher.combine(articleID) - } -} - -extension Set where Element == DatabaseArticle { - - func articleIDs() -> Set { - return Set(map { $0.articleID }) - } -} diff --git a/Frameworks/ArticlesDatabase/Extensions/Article+Database.swift b/Frameworks/ArticlesDatabase/Extensions/Article+Database.swift index 058922dfb..165887067 100644 --- a/Frameworks/ArticlesDatabase/Extensions/Article+Database.swift +++ b/Frameworks/ArticlesDatabase/Extensions/Article+Database.swift @@ -13,8 +13,31 @@ import RSParser extension Article { - init(databaseArticle: DatabaseArticle, accountID: String, authors: Set?) { - self.init(accountID: accountID, articleID: databaseArticle.articleID, webFeedID: databaseArticle.webFeedID, uniqueID: databaseArticle.uniqueID, title: databaseArticle.title, contentHTML: databaseArticle.contentHTML, contentText: databaseArticle.contentText, url: databaseArticle.url, externalURL: databaseArticle.externalURL, summary: databaseArticle.summary, imageURL: databaseArticle.imageURL, bannerImageURL: databaseArticle.bannerImageURL, datePublished: databaseArticle.datePublished, dateModified: databaseArticle.dateModified, authors: authors, status: databaseArticle.status) + init?(accountID: String, row: FMResultSet, status: ArticleStatus) { + guard let articleID = row.string(forColumn: DatabaseKey.articleID) else { + assertionFailure("Expected articleID.") + return nil + } + guard let webFeedID = row.string(forColumn: DatabaseKey.feedID) else { + assertionFailure("Expected feedID.") + return nil + } + guard let uniqueID = row.string(forColumn: DatabaseKey.uniqueID) else { + assertionFailure("Expected uniqueID.") + return nil + } + + let title = row.string(forColumn: DatabaseKey.title) + let contentHTML = row.string(forColumn: DatabaseKey.contentHTML) + let contentText = row.string(forColumn: DatabaseKey.contentText) + let url = row.string(forColumn: DatabaseKey.url) + let externalURL = row.string(forColumn: DatabaseKey.externalURL) + let summary = row.string(forColumn: DatabaseKey.summary) + let imageURL = row.string(forColumn: DatabaseKey.imageURL) + let datePublished = row.date(forColumn: DatabaseKey.datePublished) + let dateModified = row.date(forColumn: DatabaseKey.dateModified) + + self.init(accountID: accountID, articleID: articleID, webFeedID: webFeedID, uniqueID: uniqueID, title: title, contentHTML: contentHTML, contentText: contentText, url: url, externalURL: externalURL, summary: summary, imageURL: imageURL, datePublished: datePublished, dateModified: dateModified, authors: nil, status: status) } init(parsedItem: ParsedItem, maximumDateAllowed: Date, accountID: String, webFeedID: String, status: ArticleStatus) { @@ -34,7 +57,7 @@ extension Article { dateModified = nil } - self.init(accountID: accountID, articleID: parsedItem.syncServiceID, webFeedID: webFeedID, uniqueID: parsedItem.uniqueID, title: parsedItem.title, contentHTML: parsedItem.contentHTML, contentText: parsedItem.contentText, url: parsedItem.url, externalURL: parsedItem.externalURL, summary: parsedItem.summary, imageURL: parsedItem.imageURL, bannerImageURL: parsedItem.bannerImageURL, datePublished: datePublished, dateModified: dateModified, authors: authors, status: status) + self.init(accountID: accountID, articleID: parsedItem.syncServiceID, webFeedID: webFeedID, uniqueID: parsedItem.uniqueID, title: parsedItem.title, contentHTML: parsedItem.contentHTML, contentText: parsedItem.contentText, url: parsedItem.url, externalURL: parsedItem.externalURL, summary: parsedItem.summary, imageURL: parsedItem.imageURL, datePublished: datePublished, dateModified: dateModified, authors: authors, status: status) } private func addPossibleStringChangeWithKeyPath(_ comparisonKeyPath: KeyPath, _ otherArticle: Article, _ key: String, _ dictionary: inout DatabaseDictionary) { @@ -42,7 +65,14 @@ extension Article { dictionary[key] = self[keyPath: comparisonKeyPath] ?? "" } } - + + func byAdding(_ authors: Set) -> Article { + if authors.isEmpty { + return self + } + return Article(accountID: self.accountID, articleID: self.articleID, webFeedID: self.webFeedID, uniqueID: self.uniqueID, title: self.title, contentHTML: self.contentHTML, contentText: self.contentText, url: self.url, externalURL: self.externalURL, summary: self.summary, imageURL: self.imageURL, datePublished: self.datePublished, dateModified: self.dateModified, authors: authors, status: self.status) + } + func changesFrom(_ existingArticle: Article) -> DatabaseDictionary? { if self == existingArticle { return nil @@ -60,7 +90,6 @@ extension Article { addPossibleStringChangeWithKeyPath(\Article.externalURL, existingArticle, DatabaseKey.externalURL, &d) addPossibleStringChangeWithKeyPath(\Article.summary, existingArticle, DatabaseKey.summary, &d) addPossibleStringChangeWithKeyPath(\Article.imageURL, existingArticle, DatabaseKey.imageURL, &d) - addPossibleStringChangeWithKeyPath(\Article.bannerImageURL, existingArticle, DatabaseKey.bannerImageURL, &d) // If updated versions of dates are nil, and we have existing dates, keep the existing dates. // This is data that’s good to have, and it’s likely that a feed removing dates is doing so in error. @@ -120,9 +149,6 @@ extension Article: DatabaseObject { if let imageURL = imageURL { d[DatabaseKey.imageURL] = imageURL } - if let bannerImageURL = bannerImageURL { - d[DatabaseKey.bannerImageURL] = bannerImageURL - } if let datePublished = datePublished { d[DatabaseKey.datePublished] = datePublished } diff --git a/Frameworks/SyncDatabase/SyncDatabase.swift b/Frameworks/SyncDatabase/SyncDatabase.swift index 975bc5607..7790193e9 100644 --- a/Frameworks/SyncDatabase/SyncDatabase.swift +++ b/Frameworks/SyncDatabase/SyncDatabase.swift @@ -13,6 +13,9 @@ import RSDatabase public typealias SyncStatusesResult = Result, DatabaseError> public typealias SyncStatusesCompletionBlock = (SyncStatusesResult) -> Void +public typealias SyncStatusArticleIDsResult = Result, DatabaseError> +public typealias SyncStatusArticleIDsCompletionBlock = (SyncStatusArticleIDsResult) -> Void + public struct SyncDatabase { private let syncStatusTable: SyncStatusTable @@ -41,6 +44,14 @@ public struct SyncDatabase { syncStatusTable.selectPendingCount(completion) } + public func selectPendingReadStatusArticleIDs(completion: @escaping SyncStatusArticleIDsCompletionBlock) { + syncStatusTable.selectPendingReadStatusArticleIDs(completion: completion) + } + + public func selectPendingStarredStatusArticleIDs(completion: @escaping SyncStatusArticleIDsCompletionBlock) { + syncStatusTable.selectPendingStarredStatusArticleIDs(completion: completion) + } + public func resetSelectedForProcessing(_ articleIDs: [String], completion: DatabaseCompletionBlock? = nil) { syncStatusTable.resetSelectedForProcessing(articleIDs, completion: completion) } diff --git a/Frameworks/SyncDatabase/SyncStatusTable.swift b/Frameworks/SyncDatabase/SyncStatusTable.swift index 04100c783..3d93ffab1 100644 --- a/Frameworks/SyncDatabase/SyncStatusTable.swift +++ b/Frameworks/SyncDatabase/SyncStatusTable.swift @@ -83,6 +83,14 @@ struct SyncStatusTable: DatabaseTable { } } + func selectPendingReadStatusArticleIDs(completion: @escaping SyncStatusArticleIDsCompletionBlock) { + selectPendingArticleIDsAsync(.read, completion) + } + + func selectPendingStarredStatusArticleIDs(completion: @escaping SyncStatusArticleIDsCompletionBlock) { + selectPendingArticleIDsAsync(.starred, completion) + } + func resetSelectedForProcessing(_ articleIDs: [String], completion: DatabaseCompletionBlock? = nil) { queue.runInTransaction { databaseResult in @@ -156,6 +164,38 @@ private extension SyncStatusTable { return SyncStatus(articleID: articleID, key: key, flag: flag, selected: selected) } + + func selectPendingArticleIDsAsync(_ statusKey: ArticleStatus.Key, _ completion: @escaping SyncStatusArticleIDsCompletionBlock) { + + queue.runInDatabase { databaseResult in + + func makeDatabaseCall(_ database: FMDatabase) { + let sql = "select articleID from syncStatus where selected == false and key = \"\(statusKey.rawValue)\";" + + guard let resultSet = database.executeQuery(sql, withArgumentsIn: nil) else { + DispatchQueue.main.async { + completion(.success(Set())) + } + return + } + + let articleIDs = resultSet.mapToSet{ $0.string(forColumnIndex: 0) } + DispatchQueue.main.async { + completion(.success(articleIDs)) + } + } + + switch databaseResult { + case .success(let database): + makeDatabaseCall(database) + case .failure(let databaseError): + DispatchQueue.main.async { + completion(.failure(databaseError)) + } + } + } + } + } private func callCompletion(_ completion: DatabaseCompletionBlock?, _ databaseError: DatabaseError?) { diff --git a/Mac/MainWindow/Timeline/ArticlePasteboardWriter.swift b/Mac/MainWindow/Timeline/ArticlePasteboardWriter.swift index 29ec5adca..351352b16 100644 --- a/Mac/MainWindow/Timeline/ArticlePasteboardWriter.swift +++ b/Mac/MainWindow/Timeline/ArticlePasteboardWriter.swift @@ -151,7 +151,6 @@ private extension ArticlePasteboardWriter { d[Key.externalURL] = article.externalURL ?? nil d[Key.summary] = article.summary ?? nil d[Key.imageURL] = article.imageURL ?? nil - d[Key.bannerImageURL] = article.bannerImageURL ?? nil d[Key.datePublished] = article.datePublished ?? nil d[Key.dateModified] = article.dateModified ?? nil d[Key.dateArrived] = article.status.dateArrived diff --git a/Mac/MainWindow/Timeline/TimelineViewController.swift b/Mac/MainWindow/Timeline/TimelineViewController.swift index bb79897f9..b930c44a5 100644 --- a/Mac/MainWindow/Timeline/TimelineViewController.swift +++ b/Mac/MainWindow/Timeline/TimelineViewController.swift @@ -588,7 +588,7 @@ final class TimelineViewController: NSViewController, UndoableCommandRunner, Unr let longTitle = "But I must explain to you how all this mistaken idea of denouncing pleasure and praising pain was born and I will give you a complete account of the system, and expound the actual teachings of the great explorer of the truth, the master-builder of human happiness. No one rejects, dislikes, or avoids pleasure itself, because it is pleasure, but because those who do not know how to pursue pleasure rationally encounter consequences that are extremely painful. Nor again is there anyone who loves or pursues or desires to obtain pain of itself, because it is pain, but because occasionally circumstances occur in which toil and pain can procure him some great pleasure. To take a trivial example, which of us ever undertakes laborious physical exercise, except to obtain some advantage from it? But who has any right to find fault with a man who chooses to enjoy a pleasure that has no annoying consequences, or one who avoids a pain that produces no resultant pleasure?" let prototypeID = "prototype" let status = ArticleStatus(articleID: prototypeID, read: false, starred: false, userDeleted: false, dateArrived: Date()) - let prototypeArticle = Article(accountID: prototypeID, articleID: prototypeID, webFeedID: prototypeID, uniqueID: prototypeID, title: longTitle, contentHTML: nil, contentText: nil, url: nil, externalURL: nil, summary: nil, imageURL: nil, bannerImageURL: nil, datePublished: nil, dateModified: nil, authors: nil, status: status) + let prototypeArticle = Article(accountID: prototypeID, articleID: prototypeID, webFeedID: prototypeID, uniqueID: prototypeID, title: longTitle, contentHTML: nil, contentText: nil, url: nil, externalURL: nil, summary: nil, imageURL: nil, datePublished: nil, dateModified: nil, authors: nil, status: status) let prototypeCellData = TimelineCellData(article: prototypeArticle, showFeedName: showingFeedNames, feedName: "Prototype Feed Name", iconImage: nil, showIcon: false, featuredImage: nil) let height = TimelineCellLayout.height(for: 100, cellData: prototypeCellData, appearance: cellAppearance) diff --git a/NetNewsWire.xcodeproj/project.pbxproj b/NetNewsWire.xcodeproj/project.pbxproj index 3e8b9b83f..dcacef335 100644 --- a/NetNewsWire.xcodeproj/project.pbxproj +++ b/NetNewsWire.xcodeproj/project.pbxproj @@ -110,7 +110,7 @@ 51707439232AA97100A461A3 /* ShareFolderPickerController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 51707438232AA97100A461A3 /* ShareFolderPickerController.swift */; }; 517630042336215100E15FFF /* main.js in Resources */ = {isa = PBXBuildFile; fileRef = 517630032336215100E15FFF /* main.js */; }; 517630052336215100E15FFF /* main.js in Resources */ = {isa = PBXBuildFile; fileRef = 517630032336215100E15FFF /* main.js */; }; - 517630232336657E00E15FFF /* ArticleViewControllerWebViewProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = 517630222336657E00E15FFF /* ArticleViewControllerWebViewProvider.swift */; }; + 517630232336657E00E15FFF /* WebViewProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = 517630222336657E00E15FFF /* WebViewProvider.swift */; }; 5183CCD0226E1E880010922C /* NonIntrinsicLabel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5183CCCF226E1E880010922C /* NonIntrinsicLabel.swift */; }; 5183CCDA226E31A50010922C /* NonIntrinsicImageView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5183CCD9226E31A50010922C /* NonIntrinsicImageView.swift */; }; 5183CCE5226F4DFA0010922C /* RefreshInterval.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5183CCE4226F4DFA0010922C /* RefreshInterval.swift */; }; @@ -148,6 +148,7 @@ 51A9A5F32380DE530033AADF /* AddWebFeedDefaultContainer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 51A66684238075AE00CB272D /* AddWebFeedDefaultContainer.swift */; }; 51A9A5F52380F6A60033AADF /* ModalNavigationController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 51A9A5F42380F6A60033AADF /* ModalNavigationController.swift */; }; 51A9A60A2382FD240033AADF /* PoppableGestureRecognizerDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 51A9A6092382FD240033AADF /* PoppableGestureRecognizerDelegate.swift */; }; + 51AB8AB323B7F4C6008F147D /* WebViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 51AB8AB223B7F4C6008F147D /* WebViewController.swift */; }; 51B62E68233186730085F949 /* IconView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 51B62E67233186730085F949 /* IconView.swift */; }; 51BB7C272335A8E5008E8144 /* ArticleActivityItemSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = 51BB7C262335A8E5008E8144 /* ArticleActivityItemSource.swift */; }; 51BB7C312335ACDE008E8144 /* page.html in Resources */ = {isa = PBXBuildFile; fileRef = 51BB7C302335ACDE008E8144 /* page.html */; }; @@ -1292,7 +1293,7 @@ 516AE9DE2372269A007DEEAA /* IconImage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IconImage.swift; sourceTree = ""; }; 51707438232AA97100A461A3 /* ShareFolderPickerController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShareFolderPickerController.swift; sourceTree = ""; }; 517630032336215100E15FFF /* main.js */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.javascript; path = main.js; sourceTree = ""; }; - 517630222336657E00E15FFF /* ArticleViewControllerWebViewProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ArticleViewControllerWebViewProvider.swift; sourceTree = ""; }; + 517630222336657E00E15FFF /* WebViewProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WebViewProvider.swift; sourceTree = ""; }; 5183CCCF226E1E880010922C /* NonIntrinsicLabel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NonIntrinsicLabel.swift; sourceTree = ""; }; 5183CCD9226E31A50010922C /* NonIntrinsicImageView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NonIntrinsicImageView.swift; sourceTree = ""; }; 5183CCE4226F4DFA0010922C /* RefreshInterval.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RefreshInterval.swift; sourceTree = ""; }; @@ -1320,6 +1321,7 @@ 51A9A5E72380CA130033AADF /* ShareFolderPickerCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShareFolderPickerCell.swift; sourceTree = ""; }; 51A9A5F42380F6A60033AADF /* ModalNavigationController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ModalNavigationController.swift; sourceTree = ""; }; 51A9A6092382FD240033AADF /* PoppableGestureRecognizerDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PoppableGestureRecognizerDelegate.swift; sourceTree = ""; }; + 51AB8AB223B7F4C6008F147D /* WebViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WebViewController.swift; sourceTree = ""; }; 51B62E67233186730085F949 /* IconView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IconView.swift; sourceTree = ""; }; 51BB7C262335A8E5008E8144 /* ArticleActivityItemSource.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ArticleActivityItemSource.swift; sourceTree = ""; }; 51BB7C302335ACDE008E8144 /* page.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = page.html; sourceTree = ""; }; @@ -1950,7 +1952,8 @@ isa = PBXGroup; children = ( 51C4527E2265092C00C03939 /* ArticleViewController.swift */, - 517630222336657E00E15FFF /* ArticleViewControllerWebViewProvider.swift */, + 51AB8AB223B7F4C6008F147D /* WebViewController.swift */, + 517630222336657E00E15FFF /* WebViewProvider.swift */, 51102164233A7D6C0007A5F7 /* ArticleExtractorButton.swift */, 51C266E9238C334800F53014 /* ContextMenuPreviewViewController.swift */, 5142192923522B5500E07E2C /* ImageViewController.swift */, @@ -3835,7 +3838,7 @@ 511B9807237DCAC90028BCAA /* UserInfoKey.swift in Sources */, 51C45269226508F600C03939 /* MasterFeedTableViewCell.swift in Sources */, 51F85BFD2275DCA800C787DC /* SingleLineUILabelSizer.swift in Sources */, - 517630232336657E00E15FFF /* ArticleViewControllerWebViewProvider.swift in Sources */, + 517630232336657E00E15FFF /* WebViewProvider.swift in Sources */, 51E43962238037C400015C31 /* AddWebFeedFolderViewController.swift in Sources */, 51C4528F226509BD00C03939 /* UnreadFeed.swift in Sources */, 51FD413B2342BD0500880194 /* MasterTimelineUnreadCountView.swift in Sources */, @@ -3843,6 +3846,7 @@ 51D87EE12311D34700E63F03 /* ActivityType.swift in Sources */, 51C452772265091600C03939 /* MultilineUILabelSizer.swift in Sources */, 51C452A522650A2D00C03939 /* SmallIconProvider.swift in Sources */, + 51AB8AB323B7F4C6008F147D /* WebViewController.swift in Sources */, 516A09392360A2AE00EAE89B /* SettingsAccountTableViewCell.swift in Sources */, 3B3A32A5238B820900314204 /* FeedWranglerAccountViewController.swift in Sources */, 51D5948722668EFA00DFC836 /* MarkStatusCommand.swift in Sources */, diff --git a/Shared/Article Rendering/main.js b/Shared/Article Rendering/main.js index 4ecf8eb5f..0116cd6b1 100644 --- a/Shared/Article Rendering/main.js +++ b/Shared/Article Rendering/main.js @@ -8,10 +8,18 @@ function wrapFrames() { }); } -// Strip out all styling so that we have better control over layout +// Strip out color and font styling + +function stripStylesFromElement(element, propertiesToStrip) { + for (name of propertiesToStrip) { + element.style.removeProperty(name); + } +} + function stripStyles() { document.getElementsByTagName("body")[0].querySelectorAll("style, link[rel=stylesheet]").forEach(element => element.remove()); - document.getElementsByTagName("body")[0].querySelectorAll("[style]").forEach(element => element.removeAttribute("style")); + // Removing "background" and "font" will also remove properties that would be reflected in them, e.g., "background-color" and "font-family" + document.getElementsByTagName("body")[0].querySelectorAll("[style]").forEach(element => stripStylesFromElement(element, ["color", "background", "font"])); } // Convert all image locations to be absolute @@ -21,6 +29,52 @@ function convertImgSrc() { }); } +// Wrap tables in an overflow-x: auto; div +function wrapTables() { + var tables = document.querySelector("div.articleBody").getElementsByTagName("table"); + + for (table of tables) { + var wrapper = document.createElement("div"); + wrapper.className = "nnw-overflow"; + table.parentNode.insertBefore(wrapper, table); + wrapper.appendChild(table); + } +} + +// Remove some children (currently just spans) from pre elements to work around a strange clipping issue +var ElementUnwrapper = { + unwrapSelector: "span", + unwrapElement: function (element) { + var parent = element.parentNode; + var children = Array.from(element.childNodes); + + for (child of children) { + parent.insertBefore(child, element); + } + + parent.removeChild(element); + }, + // `elements` can be a selector string, an element, or a list of elements + unwrapAppropriateChildren: function (elements) { + if (typeof elements[Symbol.iterator] !== 'function') + elements = [elements]; + else if (typeof elements === "string") + elements = document.querySelectorAll(elements); + + for (element of elements) { + for (unwrap of element.querySelectorAll(this.unwrapSelector)) { + this.unwrapElement(unwrap); + } + + element.normalize() + } + } +}; + +function flattenPreElements() { + ElementUnwrapper.unwrapAppropriateChildren("div.articleBody td > pre"); +} + function reloadArticleImage() { var image = document.getElementById("nnwImageIcon"); image.src = "nnwImageIcon://"; @@ -37,8 +91,10 @@ function render(data, scrollY) { window.scrollTo(0, scrollY); wrapFrames() + wrapTables() stripStyles() convertImgSrc() - + flattenPreElements() + postRenderProcessing() } diff --git a/iOS/Add/AddFolderViewController.swift b/iOS/Add/AddFolderViewController.swift index 3d2dc0047..26d794a10 100644 --- a/iOS/Add/AddFolderViewController.swift +++ b/iOS/Add/AddFolderViewController.swift @@ -20,7 +20,22 @@ class AddFolderViewController: UITableViewController, AddContainerViewController return accounts.count > 1 } - private var accounts: [Account]! + private var accounts: [Account]! { + didSet { + if let predefinedAccount = accounts.first(where: { $0.accountID == AppDefaults.addFolderAccountID }) { + selectedAccount = predefinedAccount + } else { + selectedAccount = accounts[0] + } + } + } + + private var selectedAccount: Account! { + didSet { + guard selectedAccount != oldValue else { return } + accountLabel.text = selectedAccount.flatMap { ($0 as DisplayNameProvider).nameForDisplay } + } + } weak var delegate: AddContainerViewControllerChildDelegate? @@ -32,13 +47,11 @@ class AddFolderViewController: UITableViewController, AddContainerViewController nameTextField.delegate = self - accountLabel.text = (accounts[0] as DisplayNameProvider).nameForDisplay - if shouldDisplayPicker { accountPickerView.dataSource = self accountPickerView.delegate = self - if let index = accounts.firstIndex(where: { $0.accountID == AppDefaults.addFolderAccountID }) { + if let index = accounts.firstIndex(of: selectedAccount) { accountPickerView.selectRow(index, inComponent: 0, animated: false) } @@ -52,21 +65,26 @@ class AddFolderViewController: UITableViewController, AddContainerViewController NotificationCenter.default.addObserver(self, selector: #selector(textDidChange(_:)), name: UITextField.textDidChangeNotification, object: nameTextField) } + + private func didSelect(_ account: Account) { + AppDefaults.addFolderAccountID = account.accountID + selectedAccount = account + } func cancel() { delegate?.processingDidEnd() } func add() { - let account = accounts[accountPickerView.selectedRow(inComponent: 0)] - if let folderName = nameTextField.text { - account.addFolder(folderName) { result in - switch result { - case .success: - self.delegate?.processingDidEnd() - case .failure(let error): - self.presentError(error) - } + guard let folderName = nameTextField.text else { + return + } + selectedAccount.addFolder(folderName) { result in + switch result { + case .success: + self.delegate?.processingDidEnd() + case .failure(let error): + self.presentError(error) } } } @@ -100,8 +118,7 @@ extension AddFolderViewController: UIPickerViewDataSource, UIPickerViewDelegate } func pickerView(_ pickerView: UIPickerView, didSelectRow row: Int, inComponent component: Int) { - accountLabel.text = (accounts[row] as DisplayNameProvider).nameForDisplay - AppDefaults.addFolderAccountID = accounts[row].accountID + didSelect(accounts[row]) } } diff --git a/iOS/AppAssets.swift b/iOS/AppAssets.swift index 96cad2b37..5d7749b09 100644 --- a/iOS/AppAssets.swift +++ b/iOS/AppAssets.swift @@ -117,11 +117,11 @@ struct AppAssets { return UIImage(systemName: "asterisk.circle")! }() - static var markOlderAsReadDownImage: UIImage = { + static var markBelowAsReadImage: UIImage = { return UIImage(systemName: "arrowtriangle.down.circle")! }() - static var markOlderAsReadUpImage: UIImage = { + static var markAboveAsReadImage: UIImage = { return UIImage(systemName: "arrowtriangle.up.circle")! }() diff --git a/iOS/AppDelegate.swift b/iOS/AppDelegate.swift index 18f53e55f..27964ddf1 100644 --- a/iOS/AppDelegate.swift +++ b/iOS/AppDelegate.swift @@ -59,7 +59,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD appDelegate = self // Force lazy initialization of the web view provider so that it can warm up the queue of prepared web views - let _ = ArticleViewControllerWebViewProvider.shared + let _ = WebViewProvider.shared AccountManager.shared = AccountManager() NotificationCenter.default.addObserver(self, selector: #selector(unreadCountDidChange(_:)), name: .UnreadCountDidChange, object: nil) diff --git a/iOS/Article/ArticleViewController.swift b/iOS/Article/ArticleViewController.swift index e6dc35fd9..968e7f2cb 100644 --- a/iOS/Article/ArticleViewController.swift +++ b/iOS/Article/ArticleViewController.swift @@ -12,32 +12,20 @@ import Account import Articles import SafariServices -enum ArticleViewState: Equatable { - case noSelection - case multipleSelection - case loading - case article(Article) - case extracted(Article, ExtractedArticle) -} - class ArticleViewController: UIViewController { - private struct MessageName { - static let imageWasClicked = "imageWasClicked" - static let imageWasShown = "imageWasShown" - } - @IBOutlet private weak var nextUnreadBarButtonItem: UIBarButtonItem! @IBOutlet private weak var prevArticleBarButtonItem: UIBarButtonItem! @IBOutlet private weak var nextArticleBarButtonItem: UIBarButtonItem! @IBOutlet private weak var readBarButtonItem: UIBarButtonItem! @IBOutlet private weak var starBarButtonItem: UIBarButtonItem! @IBOutlet private weak var actionBarButtonItem: UIBarButtonItem! - @IBOutlet private weak var webViewContainer: UIView! - @IBOutlet private weak var showNavigationView: UIView! - @IBOutlet private weak var showToolbarView: UIView! - @IBOutlet private weak var showNavigationViewConstraint: NSLayoutConstraint! - @IBOutlet private weak var showToolbarViewConstraint: NSLayoutConstraint! + + private var pageViewController: UIPageViewController! + + private var currentWebViewController: WebViewController? { + return pageViewController?.viewControllers?.first as? WebViewController + } private var articleExtractorButton: ArticleExtractorButton = { let button = ArticleExtractorButton(type: .system) @@ -46,44 +34,23 @@ class ArticleViewController: UIViewController { return button }() - private var webView: WKWebView! - private lazy var contextMenuInteraction = UIContextMenuInteraction(delegate: self) private var isFullScreenAvailable: Bool { return traitCollection.userInterfaceIdiom == .phone && coordinator.isRootSplitCollapsed } - private lazy var transition = ImageTransition(controller: self) - private var clickedImageCompletion: (() -> Void)? weak var coordinator: SceneCoordinator! - var state: ArticleViewState = .noSelection { + var article: Article? { didSet { - if state != oldValue { - updateUI() - reloadHTML() + if let controller = currentWebViewController, controller.article != article { + controller.article = article + DispatchQueue.main.async { + // You have to set the view controller to clear out the UIPageViewController child controller cache. + // You also have to do it in an async call or you will get a strange assertion error. + self.pageViewController.setViewControllers([controller], direction: .forward, animated: false, completion: nil) + } } - } - } - - var restoreOffset = 0 - - var currentArticle: Article? { - switch state { - case .article(let article): - return article - case .extracted(let article, _): - return article - default: - return nil - } - } - - var articleExtractorButtonState: ArticleExtractorButtonState { - get { - return articleExtractorButton.buttonState - } - set { - articleExtractorButton.buttonState = newValue + updateUI() } } @@ -92,68 +59,47 @@ class ArticleViewController: UIViewController { return keyboardManager.keyCommands } - deinit { - if webView != nil { - webView?.evaluateJavaScript("cancelImageLoad();") - webView.configuration.userContentController.removeScriptMessageHandler(forName: MessageName.imageWasClicked) - webView.configuration.userContentController.removeScriptMessageHandler(forName: MessageName.imageWasShown) - webView.removeFromSuperview() - ArticleViewControllerWebViewProvider.shared.enqueueWebView(webView) - webView = nil - } - } - override func viewDidLoad() { super.viewDidLoad() NotificationCenter.default.addObserver(self, selector: #selector(unreadCountDidChange(_:)), name: .UnreadCountDidChange, object: nil) NotificationCenter.default.addObserver(self, selector: #selector(statusesDidChange(_:)), name: .StatusesDidChange, object: nil) - NotificationCenter.default.addObserver(self, selector: #selector(webFeedIconDidBecomeAvailable(_:)), name: .WebFeedIconDidBecomeAvailable, object: nil) - NotificationCenter.default.addObserver(self, selector: #selector(avatarDidBecomeAvailable(_:)), name: .AvatarDidBecomeAvailable, object: nil) - NotificationCenter.default.addObserver(self, selector: #selector(faviconDidBecomeAvailable(_:)), name: .FaviconDidBecomeAvailable, object: nil) - NotificationCenter.default.addObserver(self, selector: #selector(contentSizeCategoryDidChange(_:)), name: UIContentSizeCategory.didChangeNotification, object: nil) NotificationCenter.default.addObserver(self, selector: #selector(willEnterForeground(_:)), name: UIApplication.willEnterForegroundNotification, object: nil) + let fullScreenTapZone = UIView() + NSLayoutConstraint.activate([ + fullScreenTapZone.widthAnchor.constraint(equalToConstant: 150), + fullScreenTapZone.heightAnchor.constraint(equalToConstant: 44) + ]) + fullScreenTapZone.addGestureRecognizer(UITapGestureRecognizer(target: self, action: #selector(didTapNavigationBar))) + navigationItem.titleView = fullScreenTapZone + articleExtractorButton.addTarget(self, action: #selector(toggleArticleExtractor(_:)), for: .touchUpInside) toolbarItems?.insert(UIBarButtonItem(customView: articleExtractorButton), at: 6) - showNavigationView.addGestureRecognizer(UITapGestureRecognizer(target: self, action: #selector(showBars(_:)))) - showToolbarView.addGestureRecognizer(UITapGestureRecognizer(target: self, action: #selector(showBars(_:)))) + pageViewController = UIPageViewController(transitionStyle: .scroll, navigationOrientation: .horizontal, options: [:]) + pageViewController.delegate = self + pageViewController.dataSource = self - ArticleViewControllerWebViewProvider.shared.dequeueWebView() { webView in - - self.webView = webView - self.webViewContainer.addChildAndPin(webView) - - webView.translatesAutoresizingMaskIntoConstraints = false - self.webViewContainer.addSubview(webView) - NSLayoutConstraint.activate([ - self.webViewContainer.leadingAnchor.constraint(equalTo: webView.leadingAnchor), - self.webViewContainer.trailingAnchor.constraint(equalTo: webView.trailingAnchor), - self.webViewContainer.topAnchor.constraint(equalTo: webView.topAnchor), - self.webViewContainer.bottomAnchor.constraint(equalTo: webView.bottomAnchor) - ]) - - webView.navigationDelegate = self - webView.uiDelegate = self - self.configureContextMenuInteraction() - - webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasClicked) - webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasShown) - - // Even though page.html should be loaded into this webview, we have to do it again - // to work around this bug: http://www.openradar.me/22855188 - let url = Bundle.main.url(forResource: "page", withExtension: "html")! - webView.loadFileURL(url, allowingReadAccessTo: url.deletingLastPathComponent()) - - } + view.addSubview(pageViewController.view) + addChild(pageViewController!) + NSLayoutConstraint.activate([ + view.leadingAnchor.constraint(equalTo: pageViewController.view.leadingAnchor), + view.trailingAnchor.constraint(equalTo: pageViewController.view.trailingAnchor), + view.topAnchor.constraint(equalTo: pageViewController.view.topAnchor), + view.bottomAnchor.constraint(equalTo: pageViewController.view.bottomAnchor) + ]) + + let controller = createWebViewController(article) + pageViewController.setViewControllers([controller], direction: .forward, animated: false, completion: nil) + updateUI() } override func viewWillAppear(_ animated: Bool) { super.viewWillAppear(animated) if AppDefaults.articleFullscreenEnabled { - hideBars() + currentWebViewController?.hideBars() } } @@ -169,7 +115,7 @@ class ArticleViewController: UIViewController { func updateUI() { - guard let article = currentArticle else { + guard let article = article else { articleExtractorButton.isEnabled = false nextUnreadBarButtonItem.isEnabled = false prevArticleBarButtonItem.isEnabled = false @@ -197,41 +143,6 @@ class ArticleViewController: UIViewController { } - func reloadHTML() { - - let style = ArticleStylesManager.shared.currentStyle - let rendering: ArticleRenderer.Rendering - - switch state { - case .noSelection: - rendering = ArticleRenderer.noSelectionHTML(style: style) - case .multipleSelection: - rendering = ArticleRenderer.multipleSelectionHTML(style: style) - case .loading: - rendering = ArticleRenderer.loadingHTML(style: style) - case .article(let article): - rendering = ArticleRenderer.articleHTML(article: article, style: style, useImageIcon: true) - case .extracted(let article, let extractedArticle): - rendering = ArticleRenderer.articleHTML(article: article, extractedArticle: extractedArticle, style: style, useImageIcon: true) - } - - let templateData = TemplateData(style: rendering.style, body: rendering.html) - - let encoder = JSONEncoder() - var render = "error();" - if let data = try? encoder.encode(templateData) { - let json = String(data: data, encoding: .utf8)! - render = "render(\(json), \(restoreOffset));" - } - - restoreOffset = 0 - - ArticleViewControllerWebViewProvider.shared.articleIconSchemeHandler.currentArticle = currentArticle - webView?.scrollView.setZoomScale(1.0, animated: false) - webView?.evaluateJavaScript(render) - - } - // MARK: Notifications @objc dynamic func unreadCountDidChange(_ notification: Notification) { @@ -242,45 +153,33 @@ class ArticleViewController: UIViewController { guard let articleIDs = note.userInfo?[Account.UserInfoKey.articleIDs] as? Set else { return } - guard let currentArticle = currentArticle else { + guard let article = article else { return } - if articleIDs.contains(currentArticle.articleID) { + if articleIDs.contains(article.articleID) { updateUI() } } - @objc func webFeedIconDidBecomeAvailable(_ note: Notification) { - reloadArticleImage() - } - - @objc func avatarDidBecomeAvailable(_ note: Notification) { - reloadArticleImage() - } - - @objc func faviconDidBecomeAvailable(_ note: Notification) { - reloadArticleImage() - } - - @objc func contentSizeCategoryDidChange(_ note: Notification) { - reloadHTML() - } - @objc func willEnterForeground(_ note: Notification) { // The toolbar will come back on you if you don't hide it again if AppDefaults.articleFullscreenEnabled { - hideBars() + currentWebViewController?.hideBars() } } // MARK: Actions + @objc func didTapNavigationBar() { + currentWebViewController?.hideBars() + } + @objc func showBars(_ sender: Any) { - showBars() + currentWebViewController?.showBars() } @IBAction func toggleArticleExtractor(_ sender: Any) { - coordinator.toggleArticleExtractor() + currentWebViewController?.toggleArticleExtractor() } @IBAction func nextUnread(_ sender: Any) { @@ -304,7 +203,7 @@ class ArticleViewController: UIViewController { } @IBAction func showActivityDialog(_ sender: Any) { - showActivityDialog() + currentWebViewController?.showActivityDialog(popOverBarButtonItem: actionBarButtonItem) } // MARK: Keyboard Shortcuts @@ -315,339 +214,81 @@ class ArticleViewController: UIViewController { // MARK: API func focus() { - webView.becomeFirstResponder() + currentWebViewController?.focus() } func finalScrollPosition() -> CGFloat { - return webView.scrollView.contentSize.height - webView.scrollView.bounds.size.height + webView.scrollView.contentInset.bottom + return currentWebViewController?.finalScrollPosition() ?? 0.0 } func canScrollDown() -> Bool { - return webView.scrollView.contentOffset.y < finalScrollPosition() + return currentWebViewController?.canScrollDown() ?? false } func scrollPageDown() { - let scrollToY: CGFloat = { - let fullScroll = webView.scrollView.contentOffset.y + webView.scrollView.bounds.size.height - let final = finalScrollPosition() - return fullScroll < final ? fullScroll : final - }() - - let convertedPoint = self.view.convert(CGPoint(x: 0, y: 0), to: webView.scrollView) - let scrollToPoint = CGPoint(x: convertedPoint.x, y: scrollToY) - webView.scrollView.setContentOffset(scrollToPoint, animated: true) - } - - func hideClickedImage() { - webView?.evaluateJavaScript("hideClickedImage();") - } - - func showClickedImage(completion: @escaping () -> Void) { - clickedImageCompletion = completion - webView?.evaluateJavaScript("showClickedImage();") + currentWebViewController?.scrollPageDown() } func fullReload() { - if let offset = webView?.scrollView.contentOffset.y { - restoreOffset = Int(offset) - webView?.reload() + currentWebViewController?.fullReload() + } + +} + +// MARK: WebViewControllerDelegate + +extension ArticleViewController: WebViewControllerDelegate { + func webViewController(_ webViewController: WebViewController, articleExtractorButtonStateDidUpdate buttonState: ArticleExtractorButtonState) { + if webViewController === currentWebViewController { + articleExtractorButton.buttonState = buttonState } } } -// MARK: InteractiveNavigationControllerTappable +// MARK: UIPageViewControllerDataSource -extension ArticleViewController: InteractiveNavigationControllerTappable { - func didTapNavigationBar() { - hideBars() - } -} - -// MARK: UIContextMenuInteractionDelegate - -extension ArticleViewController: UIContextMenuInteractionDelegate { - func contextMenuInteraction(_ interaction: UIContextMenuInteraction, configurationForMenuAtLocation location: CGPoint) -> UIContextMenuConfiguration? { +extension ArticleViewController: UIPageViewControllerDataSource { - return UIContextMenuConfiguration(identifier: nil, previewProvider: contextMenuPreviewProvider) { [weak self] suggestedActions in - guard let self = self else { return nil } - var actions = [UIAction]() - - if let action = self.prevArticleAction() { - actions.append(action) - } - if let action = self.nextArticleAction() { - actions.append(action) - } - actions.append(self.toggleReadAction()) - actions.append(self.toggleStarredAction()) - if let action = self.nextUnreadArticleAction() { - actions.append(action) - } - actions.append(self.toggleArticleExtractorAction()) - actions.append(self.shareAction()) - - return UIMenu(title: "", children: actions) - } - } - - func contextMenuInteraction(_ interaction: UIContextMenuInteraction, willPerformPreviewActionForMenuWith configuration: UIContextMenuConfiguration, animator: UIContextMenuInteractionCommitAnimating) { - coordinator.showBrowserForCurrentArticle() - } - -} - -// MARK: WKNavigationDelegate - -extension ArticleViewController: WKNavigationDelegate { - func webView(_ webView: WKWebView, decidePolicyFor navigationAction: WKNavigationAction, decisionHandler: @escaping (WKNavigationActionPolicy) -> Void) { - - if navigationAction.navigationType == .linkActivated { - - guard let url = navigationAction.request.url else { - decisionHandler(.allow) - return - } - - let components = URLComponents(url: url, resolvingAgainstBaseURL: false) - if components?.scheme == "http" || components?.scheme == "https" { - let vc = SFSafariViewController(url: url) - present(vc, animated: true) - decisionHandler(.cancel) - } else { - decisionHandler(.allow) - } - - } else { - - decisionHandler(.allow) - + func pageViewController(_ pageViewController: UIPageViewController, viewControllerBefore viewController: UIViewController) -> UIViewController? { + guard let article = coordinator.prevArticle else { + return nil } - - } - - func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) { - self.updateUI() - self.reloadHTML() + return createWebViewController(article) } -} - -// MARK: WKUIDelegate - -extension ArticleViewController: WKUIDelegate { - func webView(_ webView: WKWebView, contextMenuForElement elementInfo: WKContextMenuElementInfo, willCommitWithAnimator animator: UIContextMenuInteractionCommitAnimating) { - // We need to have at least an unimplemented WKUIDelegate assigned to the WKWebView. This makes the - // link preview launch Safari when the link preview is tapped. In theory, you shoud be able to get - // the link from the elementInfo above and transition to SFSafariViewController instead of launching - // Safari. As the time of this writing, the link in elementInfo is always nil. ¯\_(ツ)_/¯ - } -} - -// MARK: WKScriptMessageHandler - -extension ArticleViewController: WKScriptMessageHandler { - - func userContentController(_ userContentController: WKUserContentController, didReceive message: WKScriptMessage) { - switch message.name { - case MessageName.imageWasShown: - clickedImageCompletion?() - case MessageName.imageWasClicked: - imageWasClicked(body: message.body as? String) - default: - return + func pageViewController(_ pageViewController: UIPageViewController, viewControllerAfter viewController: UIViewController) -> UIViewController? { + guard let article = coordinator.nextArticle else { + return nil } + return createWebViewController(article) } } -class WrapperScriptMessageHandler: NSObject, WKScriptMessageHandler { - - // We need to wrap a message handler to prevent a circlular reference - private weak var handler: WKScriptMessageHandler? - - init(_ handler: WKScriptMessageHandler) { - self.handler = handler +// MARK: UIPageViewControllerDelegate + +extension ArticleViewController: UIPageViewControllerDelegate { + + func pageViewController(_ pageViewController: UIPageViewController, didFinishAnimating finished: Bool, previousViewControllers: [UIViewController], transitionCompleted completed: Bool) { + guard finished, completed else { return } + guard let article = currentWebViewController?.article else { return } + coordinator.selectArticle(article) + articleExtractorButton.buttonState = currentWebViewController?.articleExtractorButtonState ?? .off } - func userContentController(_ userContentController: WKUserContentController, didReceive message: WKScriptMessage) { - handler?.userContentController(userContentController, didReceive: message) - } - -} - -// MARK: UIViewControllerTransitioningDelegate - -extension ArticleViewController: UIViewControllerTransitioningDelegate { - - func animationController(forPresented presented: UIViewController, presenting: UIViewController, source: UIViewController) -> UIViewControllerAnimatedTransitioning? { - transition.presenting = true - return transition - } - - func animationController(forDismissed dismissed: UIViewController) -> UIViewControllerAnimatedTransitioning? { - transition.presenting = false - return transition - } -} - -// MARK: JSON - -private struct TemplateData: Codable { - let style: String - let body: String -} - -private struct ImageClickMessage: Codable { - let x: Float - let y: Float - let width: Float - let height: Float - let imageURL: String } // MARK: Private private extension ArticleViewController { - func reloadArticleImage() { - webView?.evaluateJavaScript("reloadArticleImage()") - } - - func imageWasClicked(body: String?) { - guard let body = body, - let data = body.data(using: .utf8), - let clickMessage = try? JSONDecoder().decode(ImageClickMessage.self, from: data), - let range = clickMessage.imageURL.range(of: ";base64,") - else { return } - - let base64Image = String(clickMessage.imageURL.suffix(from: range.upperBound)) - if let imageData = Data(base64Encoded: base64Image), let image = UIImage(data: imageData) { - - let y = CGFloat(clickMessage.y) + webView.safeAreaInsets.top - let rect = CGRect(x: CGFloat(clickMessage.x), y: y, width: CGFloat(clickMessage.width), height: CGFloat(clickMessage.height)) - transition.originFrame = webView.convert(rect, to: nil) - - if navigationController?.navigationBar.isHidden ?? false { - transition.maskFrame = webView.convert(webView.frame, to: nil) - } else { - transition.maskFrame = webView.convert(webView.safeAreaLayoutGuide.layoutFrame, to: nil) - } - - transition.originImage = image - - coordinator.showFullScreenImage(image: image, transitioningDelegate: self) - } - } - - func showActivityDialog() { - guard let preferredLink = currentArticle?.preferredLink, let url = URL(string: preferredLink) else { - return - } - - let itemSource = ArticleActivityItemSource(url: url, subject: currentArticle!.title) - let activityViewController = UIActivityViewController(activityItems: [itemSource], applicationActivities: nil) - activityViewController.popoverPresentationController?.barButtonItem = actionBarButtonItem - present(activityViewController, animated: true) - } - - func showBars() { - if isFullScreenAvailable { - AppDefaults.articleFullscreenEnabled = false - coordinator.showStatusBar() - showNavigationViewConstraint.constant = 0 - showToolbarViewConstraint.constant = 0 - navigationController?.setNavigationBarHidden(false, animated: true) - navigationController?.setToolbarHidden(false, animated: true) - configureContextMenuInteraction() - } - } - - func hideBars() { - if isFullScreenAvailable { - AppDefaults.articleFullscreenEnabled = true - coordinator.hideStatusBar() - showNavigationViewConstraint.constant = 44.0 - showToolbarViewConstraint.constant = 44.0 - navigationController?.setNavigationBarHidden(true, animated: true) - navigationController?.setToolbarHidden(true, animated: true) - configureContextMenuInteraction() - } - } - - func configureContextMenuInteraction() { - if isFullScreenAvailable { - if navigationController?.isNavigationBarHidden ?? false { - webView?.addInteraction(contextMenuInteraction) - } else { - webView?.removeInteraction(contextMenuInteraction) - } - } - - } - - func contextMenuPreviewProvider() -> UIViewController { - let previewProvider = UIStoryboard.main.instantiateController(ofType: ContextMenuPreviewViewController.self) - previewProvider.article = currentArticle - return previewProvider - } - - func prevArticleAction() -> UIAction? { - guard coordinator.isPrevArticleAvailable else { return nil } - let title = NSLocalizedString("Previous Article", comment: "Previous Article") - return UIAction(title: title, image: AppAssets.prevArticleImage) { [weak self] action in - self?.coordinator.selectPrevArticle() - } - } - - func nextArticleAction() -> UIAction? { - guard coordinator.isNextArticleAvailable else { return nil } - let title = NSLocalizedString("Next Article", comment: "Next Article") - return UIAction(title: title, image: AppAssets.nextArticleImage) { [weak self] action in - self?.coordinator.selectNextArticle() - } - } - - func toggleReadAction() -> UIAction { - let read = currentArticle?.status.read ?? false - let title = read ? NSLocalizedString("Mark as Unread", comment: "Mark as Unread") : NSLocalizedString("Mark as Read", comment: "Mark as Read") - let readImage = read ? AppAssets.circleClosedImage : AppAssets.circleOpenImage - return UIAction(title: title, image: readImage) { [weak self] action in - self?.coordinator.toggleReadForCurrentArticle() - } - } - - func toggleStarredAction() -> UIAction { - let starred = currentArticle?.status.starred ?? false - let title = starred ? NSLocalizedString("Mark as Unstarred", comment: "Mark as Unstarred") : NSLocalizedString("Mark as Starred", comment: "Mark as Starred") - let starredImage = starred ? AppAssets.starOpenImage : AppAssets.starClosedImage - return UIAction(title: title, image: starredImage) { [weak self] action in - self?.coordinator.toggleStarredForCurrentArticle() - } - } - - func nextUnreadArticleAction() -> UIAction? { - guard coordinator.isAnyUnreadAvailable else { return nil } - let title = NSLocalizedString("Next Unread Article", comment: "Next Unread Article") - return UIAction(title: title, image: AppAssets.nextUnreadArticleImage) { [weak self] action in - self?.coordinator.selectNextUnread() - } - } - - func toggleArticleExtractorAction() -> UIAction { - let extracted = articleExtractorButton.buttonState == .on - let title = extracted ? NSLocalizedString("Show Feed Article", comment: "Show Feed Article") : NSLocalizedString("Show Reader View", comment: "Show Reader View") - let extractorImage = extracted ? AppAssets.articleExtractorOffSF : AppAssets.articleExtractorOnSF - return UIAction(title: title, image: extractorImage) { [weak self] action in - self?.coordinator.toggleArticleExtractor() - } - } - - func shareAction() -> UIAction { - let title = NSLocalizedString("Share", comment: "Share") - return UIAction(title: title, image: AppAssets.shareImage) { [weak self] action in - self?.showActivityDialog() - } + func createWebViewController(_ article: Article?) -> WebViewController { + let controller = WebViewController() + controller.coordinator = coordinator + controller.delegate = self + controller.article = article + return controller } } diff --git a/iOS/Article/ImageTransition.swift b/iOS/Article/ImageTransition.swift index 0c17a59b9..0608b3c7b 100644 --- a/iOS/Article/ImageTransition.swift +++ b/iOS/Article/ImageTransition.swift @@ -10,15 +10,15 @@ import UIKit class ImageTransition: NSObject, UIViewControllerAnimatedTransitioning { - private weak var articleController: ArticleViewController? + private weak var webViewController: WebViewController? private let duration = 0.4 var presenting = true var originFrame: CGRect! var maskFrame: CGRect! var originImage: UIImage! - init(controller: ArticleViewController) { - self.articleController = controller + init(controller: WebViewController) { + self.webViewController = controller } func transitionDuration(using transitionContext: UIViewControllerContextTransitioning?) -> TimeInterval { @@ -44,7 +44,7 @@ class ImageTransition: NSObject, UIViewControllerAnimatedTransitioning { transitionContext.containerView.backgroundColor = AppAssets.fullScreenBackgroundColor transitionContext.containerView.addSubview(imageView) - articleController?.hideClickedImage() + webViewController?.hideClickedImage() UIView.animate( withDuration: duration, @@ -93,7 +93,7 @@ class ImageTransition: NSObject, UIViewControllerAnimatedTransitioning { animations: { imageView.frame = self.originFrame }, completion: { _ in - self.articleController?.showClickedImage() { + self.webViewController?.showClickedImage() { DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { imageView.removeFromSuperview() transitionContext.completeTransition(true) diff --git a/iOS/Article/ImageViewController.swift b/iOS/Article/ImageViewController.swift index d837b4d4b..acd572a13 100644 --- a/iOS/Article/ImageViewController.swift +++ b/iOS/Article/ImageViewController.swift @@ -12,8 +12,10 @@ class ImageViewController: UIViewController { @IBOutlet weak var shareButton: UIButton! @IBOutlet weak var imageScrollView: ImageScrollView! + @IBOutlet weak var titleLabel: UILabel! var image: UIImage! + var imageTitle: String? var zoomedFrame: CGRect { return imageScrollView.zoomedFrame } @@ -26,6 +28,8 @@ class ImageViewController: UIViewController { imageScrollView.imageContentMode = .aspectFit imageScrollView.initialOffset = .center imageScrollView.display(image: image) + + titleLabel.text = imageTitle ?? "" } override func viewWillTransition(to size: CGSize, with coordinator: UIViewControllerTransitionCoordinator) { diff --git a/iOS/Article/WebViewController.swift b/iOS/Article/WebViewController.swift new file mode 100644 index 000000000..42148679a --- /dev/null +++ b/iOS/Article/WebViewController.swift @@ -0,0 +1,619 @@ +// +// WebViewController.swift +// NetNewsWire-iOS +// +// Created by Maurice Parker on 12/28/19. +// Copyright © 2019 Ranchero Software. All rights reserved. +// + +import UIKit +import WebKit +import Account +import Articles +import SafariServices + +protocol WebViewControllerDelegate: class { + func webViewController(_: WebViewController, articleExtractorButtonStateDidUpdate: ArticleExtractorButtonState) +} + +class WebViewController: UIViewController { + + private struct MessageName { + static let imageWasClicked = "imageWasClicked" + static let imageWasShown = "imageWasShown" + } + + private var topShowBarsView: UIView! + private var bottomShowBarsView: UIView! + private var topShowBarsViewConstraint: NSLayoutConstraint! + private var bottomShowBarsViewConstraint: NSLayoutConstraint! + + private var webView: WKWebView! + private lazy var contextMenuInteraction = UIContextMenuInteraction(delegate: self) + private var isFullScreenAvailable: Bool { + return traitCollection.userInterfaceIdiom == .phone && coordinator.isRootSplitCollapsed + } + private lazy var transition = ImageTransition(controller: self) + private var clickedImageCompletion: (() -> Void)? + + private var articleExtractor: ArticleExtractor? = nil + private var extractedArticle: ExtractedArticle? + private var isShowingExtractedArticle = false { + didSet { + if isShowingExtractedArticle != oldValue { + reloadHTML() + } + } + } + + var articleExtractorButtonState: ArticleExtractorButtonState = .off { + didSet { + delegate.webViewController(self, articleExtractorButtonStateDidUpdate: articleExtractorButtonState) + } + } + + weak var coordinator: SceneCoordinator! + weak var delegate: WebViewControllerDelegate! + + var article: Article? { + didSet { + stopArticleExtractor() + if article?.webFeed?.isArticleExtractorAlwaysOn ?? false { + startArticleExtractor() + } + if article != oldValue { + reloadHTML() + } + } + } + + var restoreOffset = 0 + + deinit { + if webView != nil { + webView?.evaluateJavaScript("cancelImageLoad();") + webView.configuration.userContentController.removeScriptMessageHandler(forName: MessageName.imageWasClicked) + webView.configuration.userContentController.removeScriptMessageHandler(forName: MessageName.imageWasShown) + webView.removeFromSuperview() + WebViewProvider.shared.enqueueWebView(webView) + webView = nil + } + } + + override func viewDidLoad() { + super.viewDidLoad() + + NotificationCenter.default.addObserver(self, selector: #selector(webFeedIconDidBecomeAvailable(_:)), name: .WebFeedIconDidBecomeAvailable, object: nil) + NotificationCenter.default.addObserver(self, selector: #selector(avatarDidBecomeAvailable(_:)), name: .AvatarDidBecomeAvailable, object: nil) + NotificationCenter.default.addObserver(self, selector: #selector(faviconDidBecomeAvailable(_:)), name: .FaviconDidBecomeAvailable, object: nil) + NotificationCenter.default.addObserver(self, selector: #selector(contentSizeCategoryDidChange(_:)), name: UIContentSizeCategory.didChangeNotification, object: nil) + + WebViewProvider.shared.dequeueWebView() { webView in + + // Add the webview + self.webView = webView + webView.translatesAutoresizingMaskIntoConstraints = false + self.view.addSubview(webView) + NSLayoutConstraint.activate([ + self.view.leadingAnchor.constraint(equalTo: webView.leadingAnchor), + self.view.trailingAnchor.constraint(equalTo: webView.trailingAnchor), + self.view.topAnchor.constraint(equalTo: webView.topAnchor), + self.view.bottomAnchor.constraint(equalTo: webView.bottomAnchor) + ]) + + self.configureTopShowBarsView() + self.configureBottomShowBarsView() + + // Configure the webview + webView.navigationDelegate = self + webView.uiDelegate = self + self.configureContextMenuInteraction() + + webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasClicked) + webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasShown) + + // Even though page.html should be loaded into this webview, we have to do it again + // to work around this bug: http://www.openradar.me/22855188 + let url = Bundle.main.url(forResource: "page", withExtension: "html")! + webView.loadFileURL(url, allowingReadAccessTo: url.deletingLastPathComponent()) + + self.view.setNeedsLayout() + self.view.layoutIfNeeded() + } + + } + + func reloadHTML() { + guard let webView = webView else { return } + + let style = ArticleStylesManager.shared.currentStyle + let rendering: ArticleRenderer.Rendering + + if let articleExtractor = articleExtractor, articleExtractor.state == .processing { + rendering = ArticleRenderer.loadingHTML(style: style) + } else if let article = article, let extractedArticle = extractedArticle { + if isShowingExtractedArticle { + rendering = ArticleRenderer.articleHTML(article: article, extractedArticle: extractedArticle, style: style, useImageIcon: true) + } else { + rendering = ArticleRenderer.articleHTML(article: article, style: style, useImageIcon: true) + } + } else if let article = article { + rendering = ArticleRenderer.articleHTML(article: article, style: style, useImageIcon: true) + } else { + rendering = ArticleRenderer.noSelectionHTML(style: style) + } + + let templateData = TemplateData(style: rendering.style, body: rendering.html) + + let encoder = JSONEncoder() + var render = "error();" + if let data = try? encoder.encode(templateData) { + let json = String(data: data, encoding: .utf8)! + render = "render(\(json), \(restoreOffset));" + } + + restoreOffset = 0 + + WebViewProvider.shared.articleIconSchemeHandler.currentArticle = article + webView.scrollView.setZoomScale(1.0, animated: false) + webView.evaluateJavaScript(render) + + } + + // MARK: Notifications + + @objc func webFeedIconDidBecomeAvailable(_ note: Notification) { + reloadArticleImage() + } + + @objc func avatarDidBecomeAvailable(_ note: Notification) { + reloadArticleImage() + } + + @objc func faviconDidBecomeAvailable(_ note: Notification) { + reloadArticleImage() + } + + @objc func contentSizeCategoryDidChange(_ note: Notification) { + reloadHTML() + } + + // MARK: Actions + + @objc func showBars(_ sender: Any) { + showBars() + } + + // MARK: API + + func focus() { + webView.becomeFirstResponder() + } + + func finalScrollPosition() -> CGFloat { + return webView.scrollView.contentSize.height - webView.scrollView.bounds.size.height + webView.scrollView.contentInset.bottom + } + + func canScrollDown() -> Bool { + return webView.scrollView.contentOffset.y < finalScrollPosition() + } + + func scrollPageDown() { + let scrollToY: CGFloat = { + let fullScroll = webView.scrollView.contentOffset.y + webView.scrollView.bounds.size.height + let final = finalScrollPosition() + return fullScroll < final ? fullScroll : final + }() + + let convertedPoint = self.view.convert(CGPoint(x: 0, y: 0), to: webView.scrollView) + let scrollToPoint = CGPoint(x: convertedPoint.x, y: scrollToY) + webView.scrollView.setContentOffset(scrollToPoint, animated: true) + } + + func hideClickedImage() { + webView?.evaluateJavaScript("hideClickedImage();") + } + + func showClickedImage(completion: @escaping () -> Void) { + clickedImageCompletion = completion + webView?.evaluateJavaScript("showClickedImage();") + } + + func fullReload() { + if let offset = webView?.scrollView.contentOffset.y { + restoreOffset = Int(offset) + webView?.reload() + } + } + + func showBars() { + if isFullScreenAvailable { + AppDefaults.articleFullscreenEnabled = false + coordinator.showStatusBar() + topShowBarsViewConstraint.constant = 0 + bottomShowBarsViewConstraint.constant = 0 + navigationController?.setNavigationBarHidden(false, animated: true) + navigationController?.setToolbarHidden(false, animated: true) + configureContextMenuInteraction() + } + } + + func hideBars() { + if isFullScreenAvailable { + AppDefaults.articleFullscreenEnabled = true + coordinator.hideStatusBar() + topShowBarsViewConstraint.constant = -44.0 + bottomShowBarsViewConstraint.constant = 44.0 + navigationController?.setNavigationBarHidden(true, animated: true) + navigationController?.setToolbarHidden(true, animated: true) + configureContextMenuInteraction() + } + } + + func toggleArticleExtractor() { + + guard let article = article else { + return + } + + guard articleExtractor?.state != .processing else { + stopArticleExtractor() + return + } + + guard !isShowingExtractedArticle else { + isShowingExtractedArticle = false + articleExtractorButtonState = .off + return + } + + if let articleExtractor = articleExtractor { + if article.preferredLink == articleExtractor.articleLink { + isShowingExtractedArticle = true + articleExtractorButtonState = .on + } + } else { + startArticleExtractor() + } + + } + + func showActivityDialog(popOverBarButtonItem: UIBarButtonItem? = nil) { + guard let preferredLink = article?.preferredLink, let url = URL(string: preferredLink) else { + return + } + + let itemSource = ArticleActivityItemSource(url: url, subject: article!.title) + let activityViewController = UIActivityViewController(activityItems: [itemSource], applicationActivities: nil) + activityViewController.popoverPresentationController?.barButtonItem = popOverBarButtonItem + present(activityViewController, animated: true) + } + +} + +// MARK: ArticleExtractorDelegate + +extension WebViewController: ArticleExtractorDelegate { + + func articleExtractionDidFail(with: Error) { + stopArticleExtractor() + articleExtractorButtonState = .error + } + + func articleExtractionDidComplete(extractedArticle: ExtractedArticle) { + if articleExtractor?.state != .cancelled { + self.extractedArticle = extractedArticle + isShowingExtractedArticle = true + articleExtractorButtonState = .on + } + } + +} + +// MARK: UIContextMenuInteractionDelegate + +extension WebViewController: UIContextMenuInteractionDelegate { + func contextMenuInteraction(_ interaction: UIContextMenuInteraction, configurationForMenuAtLocation location: CGPoint) -> UIContextMenuConfiguration? { + + return UIContextMenuConfiguration(identifier: nil, previewProvider: contextMenuPreviewProvider) { [weak self] suggestedActions in + guard let self = self else { return nil } + var actions = [UIAction]() + + if let action = self.prevArticleAction() { + actions.append(action) + } + if let action = self.nextArticleAction() { + actions.append(action) + } + actions.append(self.toggleReadAction()) + actions.append(self.toggleStarredAction()) + if let action = self.nextUnreadArticleAction() { + actions.append(action) + } + actions.append(self.toggleArticleExtractorAction()) + actions.append(self.shareAction()) + + return UIMenu(title: "", children: actions) + } + } + + func contextMenuInteraction(_ interaction: UIContextMenuInteraction, willPerformPreviewActionForMenuWith configuration: UIContextMenuConfiguration, animator: UIContextMenuInteractionCommitAnimating) { + coordinator.showBrowserForCurrentArticle() + } + +} + +// MARK: WKNavigationDelegate + +extension WebViewController: WKNavigationDelegate { + func webView(_ webView: WKWebView, decidePolicyFor navigationAction: WKNavigationAction, decisionHandler: @escaping (WKNavigationActionPolicy) -> Void) { + + if navigationAction.navigationType == .linkActivated { + + guard let url = navigationAction.request.url else { + decisionHandler(.allow) + return + } + + let components = URLComponents(url: url, resolvingAgainstBaseURL: false) + if components?.scheme == "http" || components?.scheme == "https" { + let vc = SFSafariViewController(url: url) + present(vc, animated: true) + decisionHandler(.cancel) + } else { + decisionHandler(.allow) + } + + } else { + + decisionHandler(.allow) + + } + + } + + func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) { + self.reloadHTML() + } + +} + +// MARK: WKUIDelegate + +extension WebViewController: WKUIDelegate { + func webView(_ webView: WKWebView, contextMenuForElement elementInfo: WKContextMenuElementInfo, willCommitWithAnimator animator: UIContextMenuInteractionCommitAnimating) { + // We need to have at least an unimplemented WKUIDelegate assigned to the WKWebView. This makes the + // link preview launch Safari when the link preview is tapped. In theory, you shoud be able to get + // the link from the elementInfo above and transition to SFSafariViewController instead of launching + // Safari. As the time of this writing, the link in elementInfo is always nil. ¯\_(ツ)_/¯ + } +} + +// MARK: WKScriptMessageHandler + +extension WebViewController: WKScriptMessageHandler { + + func userContentController(_ userContentController: WKUserContentController, didReceive message: WKScriptMessage) { + switch message.name { + case MessageName.imageWasShown: + clickedImageCompletion?() + case MessageName.imageWasClicked: + imageWasClicked(body: message.body as? String) + default: + return + } + } + +} + +class WrapperScriptMessageHandler: NSObject, WKScriptMessageHandler { + + // We need to wrap a message handler to prevent a circlular reference + private weak var handler: WKScriptMessageHandler? + + init(_ handler: WKScriptMessageHandler) { + self.handler = handler + } + + func userContentController(_ userContentController: WKUserContentController, didReceive message: WKScriptMessage) { + handler?.userContentController(userContentController, didReceive: message) + } + +} + +// MARK: UIViewControllerTransitioningDelegate + +extension WebViewController: UIViewControllerTransitioningDelegate { + + func animationController(forPresented presented: UIViewController, presenting: UIViewController, source: UIViewController) -> UIViewControllerAnimatedTransitioning? { + transition.presenting = true + return transition + } + + func animationController(forDismissed dismissed: UIViewController) -> UIViewControllerAnimatedTransitioning? { + transition.presenting = false + return transition + } +} + +// MARK: JSON + +private struct TemplateData: Codable { + let style: String + let body: String +} + +private struct ImageClickMessage: Codable { + let x: Float + let y: Float + let width: Float + let height: Float + let imageTitle: String? + let imageURL: String +} + +// MARK: Private + +private extension WebViewController { + + func startArticleExtractor() { + if let link = article?.preferredLink, let extractor = ArticleExtractor(link) { + extractor.delegate = self + extractor.process() + articleExtractor = extractor + articleExtractorButtonState = .animated + } + } + + func stopArticleExtractor() { + articleExtractor?.cancel() + articleExtractor = nil + isShowingExtractedArticle = false + articleExtractorButtonState = .off + } + + func reloadArticleImage() { + webView?.evaluateJavaScript("reloadArticleImage()") + } + + func imageWasClicked(body: String?) { + guard let body = body, + let data = body.data(using: .utf8), + let clickMessage = try? JSONDecoder().decode(ImageClickMessage.self, from: data), + let range = clickMessage.imageURL.range(of: ";base64,") + else { return } + + let base64Image = String(clickMessage.imageURL.suffix(from: range.upperBound)) + if let imageData = Data(base64Encoded: base64Image), let image = UIImage(data: imageData) { + + let y = CGFloat(clickMessage.y) + webView.safeAreaInsets.top + let rect = CGRect(x: CGFloat(clickMessage.x), y: y, width: CGFloat(clickMessage.width), height: CGFloat(clickMessage.height)) + transition.originFrame = webView.convert(rect, to: nil) + + if navigationController?.navigationBar.isHidden ?? false { + transition.maskFrame = webView.convert(webView.frame, to: nil) + } else { + transition.maskFrame = webView.convert(webView.safeAreaLayoutGuide.layoutFrame, to: nil) + } + + transition.originImage = image + + coordinator.showFullScreenImage(image: image, imageTitle: clickMessage.imageTitle, transitioningDelegate: self) + } + } + + func configureTopShowBarsView() { + topShowBarsView = UIView() + topShowBarsView.backgroundColor = .clear + topShowBarsView.translatesAutoresizingMaskIntoConstraints = false + view.addSubview(topShowBarsView) + + if AppDefaults.articleFullscreenEnabled { + topShowBarsViewConstraint = view.topAnchor.constraint(equalTo: topShowBarsView.bottomAnchor, constant: -44.0) + } else { + topShowBarsViewConstraint = view.topAnchor.constraint(equalTo: topShowBarsView.bottomAnchor, constant: 0.0) + } + + NSLayoutConstraint.activate([ + topShowBarsViewConstraint, + view.leadingAnchor.constraint(equalTo: topShowBarsView.leadingAnchor), + view.trailingAnchor.constraint(equalTo: topShowBarsView.trailingAnchor), + topShowBarsView.heightAnchor.constraint(equalToConstant: 44.0) + ]) + topShowBarsView.addGestureRecognizer(UITapGestureRecognizer(target: self, action: #selector(showBars(_:)))) + } + + func configureBottomShowBarsView() { + bottomShowBarsView = UIView() + topShowBarsView.backgroundColor = .clear + bottomShowBarsView.translatesAutoresizingMaskIntoConstraints = false + view.addSubview(bottomShowBarsView) + if AppDefaults.articleFullscreenEnabled { + bottomShowBarsViewConstraint = view.bottomAnchor.constraint(equalTo: bottomShowBarsView.topAnchor, constant: 44.0) + } else { + bottomShowBarsViewConstraint = view.bottomAnchor.constraint(equalTo: bottomShowBarsView.topAnchor, constant: 0.0) + } + NSLayoutConstraint.activate([ + bottomShowBarsViewConstraint, + view.leadingAnchor.constraint(equalTo: bottomShowBarsView.leadingAnchor), + view.trailingAnchor.constraint(equalTo: bottomShowBarsView.trailingAnchor), + bottomShowBarsView.heightAnchor.constraint(equalToConstant: 44.0) + ]) + bottomShowBarsView.addGestureRecognizer(UITapGestureRecognizer(target: self, action: #selector(showBars(_:)))) + } + + func configureContextMenuInteraction() { + if isFullScreenAvailable { + if navigationController?.isNavigationBarHidden ?? false { + webView?.addInteraction(contextMenuInteraction) + } else { + webView?.removeInteraction(contextMenuInteraction) + } + } + } + + func contextMenuPreviewProvider() -> UIViewController { + let previewProvider = UIStoryboard.main.instantiateController(ofType: ContextMenuPreviewViewController.self) + previewProvider.article = article + return previewProvider + } + + func prevArticleAction() -> UIAction? { + guard coordinator.isPrevArticleAvailable else { return nil } + let title = NSLocalizedString("Previous Article", comment: "Previous Article") + return UIAction(title: title, image: AppAssets.prevArticleImage) { [weak self] action in + self?.coordinator.selectPrevArticle() + } + } + + func nextArticleAction() -> UIAction? { + guard coordinator.isNextArticleAvailable else { return nil } + let title = NSLocalizedString("Next Article", comment: "Next Article") + return UIAction(title: title, image: AppAssets.nextArticleImage) { [weak self] action in + self?.coordinator.selectNextArticle() + } + } + + func toggleReadAction() -> UIAction { + let read = article?.status.read ?? false + let title = read ? NSLocalizedString("Mark as Unread", comment: "Mark as Unread") : NSLocalizedString("Mark as Read", comment: "Mark as Read") + let readImage = read ? AppAssets.circleClosedImage : AppAssets.circleOpenImage + return UIAction(title: title, image: readImage) { [weak self] action in + self?.coordinator.toggleReadForCurrentArticle() + } + } + + func toggleStarredAction() -> UIAction { + let starred = article?.status.starred ?? false + let title = starred ? NSLocalizedString("Mark as Unstarred", comment: "Mark as Unstarred") : NSLocalizedString("Mark as Starred", comment: "Mark as Starred") + let starredImage = starred ? AppAssets.starOpenImage : AppAssets.starClosedImage + return UIAction(title: title, image: starredImage) { [weak self] action in + self?.coordinator.toggleStarredForCurrentArticle() + } + } + + func nextUnreadArticleAction() -> UIAction? { + guard coordinator.isAnyUnreadAvailable else { return nil } + let title = NSLocalizedString("Next Unread Article", comment: "Next Unread Article") + return UIAction(title: title, image: AppAssets.nextUnreadArticleImage) { [weak self] action in + self?.coordinator.selectNextUnread() + } + } + + func toggleArticleExtractorAction() -> UIAction { + let extracted = articleExtractorButtonState == .on + let title = extracted ? NSLocalizedString("Show Feed Article", comment: "Show Feed Article") : NSLocalizedString("Show Reader View", comment: "Show Reader View") + let extractorImage = extracted ? AppAssets.articleExtractorOffSF : AppAssets.articleExtractorOnSF + return UIAction(title: title, image: extractorImage) { [weak self] action in + self?.toggleArticleExtractor() + } + } + + func shareAction() -> UIAction { + let title = NSLocalizedString("Share", comment: "Share") + return UIAction(title: title, image: AppAssets.shareImage) { [weak self] action in + self?.showActivityDialog() + } + } + +} diff --git a/iOS/Article/ArticleViewControllerWebViewProvider.swift b/iOS/Article/WebViewProvider.swift similarity index 93% rename from iOS/Article/ArticleViewControllerWebViewProvider.swift rename to iOS/Article/WebViewProvider.swift index d8c34aad5..2c90e4505 100644 --- a/iOS/Article/ArticleViewControllerWebViewProvider.swift +++ b/iOS/Article/WebViewProvider.swift @@ -1,5 +1,5 @@ // -// ArticleViewControllerWebViewProvider.swift +// WebViewProvider.swift // NetNewsWire-iOS // // Created by Maurice Parker on 9/21/19. @@ -11,9 +11,9 @@ import WebKit /// WKWebView has an awful behavior of a flash to white on first load when in dark mode. /// Keep a queue of WebViews where we've already done a trivial load so that by the time we need them in the UI, they're past the flash-to-shite part of their lifecycle. -class ArticleViewControllerWebViewProvider: NSObject, WKNavigationDelegate { +class WebViewProvider: NSObject, WKNavigationDelegate { - static let shared = ArticleViewControllerWebViewProvider() + static let shared = WebViewProvider() let articleIconSchemeHandler = ArticleIconSchemeHandler() diff --git a/iOS/ArticleActivityItemSource.swift b/iOS/ArticleActivityItemSource.swift index ba168651e..19177615f 100644 --- a/iOS/ArticleActivityItemSource.swift +++ b/iOS/ArticleActivityItemSource.swift @@ -23,7 +23,18 @@ class ArticleActivityItemSource: NSObject, UIActivityItemSource { } func activityViewController(_ activityViewController: UIActivityViewController, itemForActivityType activityType: UIActivity.ActivityType?) -> Any? { - return url + guard let activityType = activityType, + let subject = subject else { + return url + } + + switch activityType.rawValue { + case "com.omnigroup.OmniFocus3.iOS.QuickEntry", + "com.culturedcode.ThingsiPhone.ShareExtension": + return "\(subject)\n\(url)" + default: + return url + } } func activityViewController(_ activityViewController: UIActivityViewController, subjectForActivityType activityType: UIActivity.ActivityType?) -> String { diff --git a/iOS/Base.lproj/Main.storyboard b/iOS/Base.lproj/Main.storyboard index c16a87a55..e579a5ab0 100644 --- a/iOS/Base.lproj/Main.storyboard +++ b/iOS/Base.lproj/Main.storyboard @@ -15,39 +15,7 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -75,7 +43,7 @@ - + @@ -117,15 +85,10 @@ - + - - - - - @@ -156,13 +119,20 @@ - + + + + + - + + + + @@ -251,6 +221,12 @@ +