Merge pull request #1619 from kielgillard/feedly-article-status-switching

Fix read articles switching back to unread in Feedly
This commit is contained in:
Brent Simmons 2020-01-13 11:44:01 -08:00 committed by GitHub
commit 971abee39d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 76 additions and 22 deletions

View File

@ -89,6 +89,7 @@ class FeedlyAddNewFeedOperationTests: XCTestCase {
}
let progress = DownloadProgress(numberOfTasks: 0)
let container = support.makeTestDatabaseContainer()
let _ = expectationForCompletion(of: progress)
let addNewFeed = try! FeedlyAddNewFeedOperation(account: account,
@ -99,6 +100,7 @@ class FeedlyAddNewFeedOperationTests: XCTestCase {
addToCollectionService: caller,
syncUnreadIdsService: caller,
getStreamContentsService: caller,
database: container.database,
container: folder,
progress: progress,
log: support.log)
@ -126,6 +128,7 @@ class FeedlyAddNewFeedOperationTests: XCTestCase {
}
let progress = DownloadProgress(numberOfTasks: 0)
let container = support.makeTestDatabaseContainer()
let _ = expectationForCompletion(of: progress)
let subdirectory = "feedly-add-new-feed"
@ -145,6 +148,7 @@ class FeedlyAddNewFeedOperationTests: XCTestCase {
addToCollectionService: caller,
syncUnreadIdsService: caller,
getStreamContentsService: caller,
database: container.database,
container: folder,
progress: progress,
log: support.log)
@ -191,6 +195,7 @@ class FeedlyAddNewFeedOperationTests: XCTestCase {
}
let progress = DownloadProgress(numberOfTasks: 0)
let container = support.makeTestDatabaseContainer()
let _ = expectationForCompletion(of: progress)
let subdirectory = "feedly-add-new-feed"
@ -220,6 +225,7 @@ class FeedlyAddNewFeedOperationTests: XCTestCase {
addToCollectionService: service,
syncUnreadIdsService: caller,
getStreamContentsService: caller,
database: container.database,
container: folder,
progress: progress,
log: support.log)

View File

@ -156,7 +156,7 @@ final class FeedlyAccountDelegate: AccountDelegate {
let group = DispatchGroup()
let ingestUnread = FeedlyIngestUnreadArticleIdsOperation(account: account, credentials: credentials, service: caller, newerThan: nil, log: log)
let ingestUnread = FeedlyIngestUnreadArticleIdsOperation(account: account, credentials: credentials, service: caller, database: database, newerThan: nil, log: log)
group.enter()
ingestUnread.completionBlock = {
@ -164,7 +164,7 @@ final class FeedlyAccountDelegate: AccountDelegate {
}
let ingestStarred = FeedlyIngestStarredArticleIdsOperation(account: account, credentials: credentials, service: caller, newerThan: nil, log: log)
let ingestStarred = FeedlyIngestStarredArticleIdsOperation(account: account, credentials: credentials, service: caller, database: database, newerThan: nil, log: log)
group.enter()
ingestStarred.completionBlock = {
@ -297,6 +297,7 @@ final class FeedlyAccountDelegate: AccountDelegate {
addToCollectionService: caller,
syncUnreadIdsService: caller,
getStreamContentsService: caller,
database: database,
container: container,
progress: refreshProgress,
log: log)

View File

@ -8,6 +8,7 @@
import Foundation
import os.log
import SyncDatabase
import RSWeb
class FeedlyAddNewFeedOperation: FeedlyOperation, FeedlyOperationDelegate, FeedlySearchOperationDelegate, FeedlyCheckpointOperationDelegate {
@ -17,6 +18,7 @@ class FeedlyAddNewFeedOperation: FeedlyOperation, FeedlyOperationDelegate, Feedl
private let url: String
private let account: Account
private let credentials: Credentials
private let database: SyncDatabase
private let feedName: String?
private let addToCollectionService: FeedlyAddFeedToCollectionService
private let syncUnreadIdsService: FeedlyGetStreamIdsService
@ -25,7 +27,7 @@ class FeedlyAddNewFeedOperation: FeedlyOperation, FeedlyOperationDelegate, Feedl
var addCompletionHandler: ((Result<WebFeed, Error>) -> ())?
init(account: Account, credentials: Credentials, url: String, feedName: String?, searchService: FeedlySearchService, addToCollectionService: FeedlyAddFeedToCollectionService, syncUnreadIdsService: FeedlyGetStreamIdsService, getStreamContentsService: FeedlyGetStreamContentsService, container: Container, progress: DownloadProgress, log: OSLog) throws {
init(account: Account, credentials: Credentials, url: String, feedName: String?, searchService: FeedlySearchService, addToCollectionService: FeedlyAddFeedToCollectionService, syncUnreadIdsService: FeedlyGetStreamIdsService, getStreamContentsService: FeedlyGetStreamContentsService, database: SyncDatabase, container: Container, progress: DownloadProgress, log: OSLog) throws {
let validator = FeedlyFeedContainerValidator(container: container, userId: credentials.username)
(self.folder, self.collectionId) = try validator.getValidContainer()
@ -35,6 +37,7 @@ class FeedlyAddNewFeedOperation: FeedlyOperation, FeedlyOperationDelegate, Feedl
self.operationQueue.isSuspended = true
self.account = account
self.credentials = credentials
self.database = database
self.feedName = feedName
self.addToCollectionService = addToCollectionService
self.syncUnreadIdsService = syncUnreadIdsService
@ -92,7 +95,7 @@ class FeedlyAddNewFeedOperation: FeedlyOperation, FeedlyOperationDelegate, Feedl
createFeeds.downloadProgress = downloadProgress
self.operationQueue.addOperation(createFeeds)
let syncUnread = FeedlyIngestUnreadArticleIdsOperation(account: account, credentials: credentials, service: syncUnreadIdsService, newerThan: nil, log: log)
let syncUnread = FeedlyIngestUnreadArticleIdsOperation(account: account, credentials: credentials, service: syncUnreadIdsService, database: database, newerThan: nil, log: log)
syncUnread.addDependency(createFeeds)
syncUnread.downloadProgress = downloadProgress
self.operationQueue.addOperation(syncUnread)

View File

@ -8,6 +8,7 @@
import Foundation
import os.log
import SyncDatabase
/// Single responsibility is to clone locally the remote starred article state.
///
@ -19,19 +20,20 @@ final class FeedlyIngestStarredArticleIdsOperation: FeedlyOperation {
private let account: Account
private let resource: FeedlyResourceId
private let service: FeedlyGetStreamIdsService
private let entryIdsProvider: FeedlyEntryIdentifierProvider
private let database: SyncDatabase
private var remoteEntryIds = Set<String>()
private let log: OSLog
convenience init(account: Account, credentials: Credentials, service: FeedlyGetStreamIdsService, newerThan: Date?, log: OSLog) {
convenience init(account: Account, credentials: Credentials, service: FeedlyGetStreamIdsService, database: SyncDatabase, newerThan: Date?, log: OSLog) {
let resource = FeedlyTagResourceId.Global.saved(for: credentials.username)
self.init(account: account, resource: resource, service: service, newerThan: newerThan, log: log)
self.init(account: account, resource: resource, service: service, database: database, newerThan: newerThan, log: log)
}
init(account: Account, resource: FeedlyResourceId, service: FeedlyGetStreamIdsService, newerThan: Date?, log: OSLog) {
init(account: Account, resource: FeedlyResourceId, service: FeedlyGetStreamIdsService, database: SyncDatabase, newerThan: Date?, log: OSLog) {
self.account = account
self.resource = resource
self.service = service
self.entryIdsProvider = FeedlyEntryIdentifierProvider()
self.database = database
self.log = log
}
@ -57,10 +59,10 @@ final class FeedlyIngestStarredArticleIdsOperation: FeedlyOperation {
switch result {
case .success(let streamIds):
entryIdsProvider.addEntryIds(in: streamIds.ids)
remoteEntryIds.formUnion(streamIds.ids)
guard let continuation = streamIds.continuation else {
updateStarredStatuses()
removeEntryIdsWithPendingStatus()
return
}
@ -71,6 +73,26 @@ final class FeedlyIngestStarredArticleIdsOperation: FeedlyOperation {
}
}
/// Do not override pending statuses with the remote statuses of the same articles, otherwise an article will temporarily re-acquire the remote status before the pending status is pushed and subseqently pulled.
private func removeEntryIdsWithPendingStatus() {
guard !isCancelled else {
didFinish()
return
}
database.selectPendingStarredStatusArticleIDs { result in
switch result {
case .success(let pendingArticleIds):
self.remoteEntryIds.subtract(pendingArticleIds)
self.updateStarredStatuses()
case .failure(let error):
self.didFinish(error)
}
}
}
private func updateStarredStatuses() {
guard !isCancelled else {
didFinish()
@ -94,7 +116,7 @@ final class FeedlyIngestStarredArticleIdsOperation: FeedlyOperation {
return
}
let remoteStarredArticleIDs = entryIdsProvider.entryIds
let remoteStarredArticleIDs = remoteEntryIds
let group = DispatchGroup()

View File

@ -9,6 +9,7 @@
import Foundation
import os.log
import RSParser
import SyncDatabase
/// Single responsibility is to clone locally the remote unread article state.
///
@ -20,19 +21,20 @@ final class FeedlyIngestUnreadArticleIdsOperation: FeedlyOperation {
private let account: Account
private let resource: FeedlyResourceId
private let service: FeedlyGetStreamIdsService
private let entryIdsProvider: FeedlyEntryIdentifierProvider
private let database: SyncDatabase
private var remoteEntryIds = Set<String>()
private let log: OSLog
convenience init(account: Account, credentials: Credentials, service: FeedlyGetStreamIdsService, newerThan: Date?, log: OSLog) {
convenience init(account: Account, credentials: Credentials, service: FeedlyGetStreamIdsService, database: SyncDatabase, newerThan: Date?, log: OSLog) {
let resource = FeedlyCategoryResourceId.Global.all(for: credentials.username)
self.init(account: account, resource: resource, service: service, newerThan: newerThan, log: log)
self.init(account: account, resource: resource, service: service, database: database, newerThan: newerThan, log: log)
}
init(account: Account, resource: FeedlyResourceId, service: FeedlyGetStreamIdsService, newerThan: Date?, log: OSLog) {
init(account: Account, resource: FeedlyResourceId, service: FeedlyGetStreamIdsService, database: SyncDatabase, newerThan: Date?, log: OSLog) {
self.account = account
self.resource = resource
self.service = service
self.entryIdsProvider = FeedlyEntryIdentifierProvider()
self.database = database
self.log = log
}
@ -58,10 +60,10 @@ final class FeedlyIngestUnreadArticleIdsOperation: FeedlyOperation {
switch result {
case .success(let streamIds):
entryIdsProvider.addEntryIds(in: streamIds.ids)
remoteEntryIds.formUnion(streamIds.ids)
guard let continuation = streamIds.continuation else {
updateUnreadStatuses()
removeEntryIdsWithPendingStatus()
return
}
@ -72,6 +74,26 @@ final class FeedlyIngestUnreadArticleIdsOperation: FeedlyOperation {
}
}
/// Do not override pending statuses with the remote statuses of the same articles, otherwise an article will temporarily re-acquire the remote status before the pending status is pushed and subseqently pulled.
private func removeEntryIdsWithPendingStatus() {
guard !isCancelled else {
didFinish()
return
}
database.selectPendingReadStatusArticleIDs { result in
switch result {
case .success(let pendingArticleIds):
self.remoteEntryIds.subtract(pendingArticleIds)
self.updateUnreadStatuses()
case .failure(let error):
self.didFinish(error)
}
}
}
private func updateUnreadStatuses() {
guard !isCancelled else {
didFinish()
@ -95,7 +117,7 @@ final class FeedlyIngestUnreadArticleIdsOperation: FeedlyOperation {
return
}
let remoteUnreadArticleIDs = entryIdsProvider.entryIds
let remoteUnreadArticleIDs = remoteEntryIds
let group = DispatchGroup()
final class ReadStatusResults {

View File

@ -73,7 +73,7 @@ final class FeedlySyncAllOperation: FeedlyOperation {
self.operationQueue.addOperation(getAllArticleIds)
// Get each page of unread article ids in the global.all stream for the last 31 days (nil = Feedly API default).
let getUnread = FeedlyIngestUnreadArticleIdsOperation(account: account, credentials: credentials, service: getUnreadService, newerThan: nil, log: log)
let getUnread = FeedlyIngestUnreadArticleIdsOperation(account: account, credentials: credentials, service: getUnreadService, database: database, newerThan: nil, log: log)
getUnread.delegate = self
getUnread.addDependency(getAllArticleIds)
getUnread.downloadProgress = downloadProgress
@ -88,7 +88,7 @@ final class FeedlySyncAllOperation: FeedlyOperation {
self.operationQueue.addOperation(getUpdated)
// Get each page of the article ids for starred articles.
let getStarred = FeedlyIngestStarredArticleIdsOperation(account: account, credentials: credentials, service: getStarredService, newerThan: nil, log: log)
let getStarred = FeedlyIngestStarredArticleIdsOperation(account: account, credentials: credentials, service: getStarredService, database: database, newerThan: nil, log: log)
getStarred.delegate = self
getStarred.downloadProgress = downloadProgress
getStarred.addDependency(createFeedsOperation)