diff --git a/Frameworks/Account/Feedly/FeedlyAPICaller.swift b/Frameworks/Account/Feedly/FeedlyAPICaller.swift index 4d93167af..d6dfbd6b3 100644 --- a/Frameworks/Account/Feedly/FeedlyAPICaller.swift +++ b/Frameworks/Account/Feedly/FeedlyAPICaller.swift @@ -89,12 +89,7 @@ final class FeedlyAPICaller { } } - func getStream(for collection: FeedlyCollection, newerThan: Date? = nil, unreadOnly: Bool? = nil, completionHandler: @escaping (Result) -> ()) { - let id = FeedlyCategoryResourceId(id: collection.id) - getStream(for: id, newerThan: newerThan, unreadOnly: unreadOnly, completionHandler: completionHandler) - } - - func getStream(for resource: FeedlyResourceId, newerThan: Date?, unreadOnly: Bool?, completionHandler: @escaping (Result) -> ()) { + func getStream(for resource: FeedlyResourceId, continuation: String? = nil, newerThan: Date?, unreadOnly: Bool?, completionHandler: @escaping (Result) -> ()) { guard let accessToken = credentials?.secret else { return DispatchQueue.main.async { completionHandler(.failure(CredentialsError.incompleteCredentials)) @@ -103,7 +98,7 @@ final class FeedlyAPICaller { var components = baseUrlComponents components.path = "/v3/streams/contents" - // If you change these, check AccountFeedlySyncTest.set(testFiles:with:). + var queryItems = [URLQueryItem]() if let date = newerThan { @@ -118,6 +113,11 @@ final class FeedlyAPICaller { queryItems.append(queryItem) } + if let value = continuation, !value.isEmpty { + let queryItem = URLQueryItem(name: "continuation", value: value) + queryItems.append(queryItem) + } + queryItems.append(contentsOf: [ URLQueryItem(name: "count", value: "1000"), URLQueryItem(name: "streamId", value: resource.id), diff --git a/Frameworks/Account/Feedly/FeedlyCompoundOperation.swift b/Frameworks/Account/Feedly/FeedlyCompoundOperation.swift index 06033e8cb..d15c9a8ff 100644 --- a/Frameworks/Account/Feedly/FeedlyCompoundOperation.swift +++ b/Frameworks/Account/Feedly/FeedlyCompoundOperation.swift @@ -11,11 +11,28 @@ import Foundation /// An operation with a queue of its own. final class FeedlyCompoundOperation: FeedlyOperation { private let operationQueue = OperationQueue() - private let operations: [Operation] + private var finishOperation: BlockOperation? init(operations: [Operation]) { assert(!operations.isEmpty) - self.operations = operations + operationQueue.isSuspended = true + finishOperation = nil + super.init() + + let finish = BlockOperation { + self.didFinish() + } + + finishOperation = finish + + for operation in operations { + finish.addDependency(operation) + } + + var initialOperations = operations + initialOperations.append(finish) + + operationQueue.addOperations(initialOperations, waitUntilFinished: false) } convenience init(operationsBlock: () -> ([Operation])) { @@ -24,18 +41,17 @@ final class FeedlyCompoundOperation: FeedlyOperation { } override func main() { - let finishOperation = BlockOperation { [weak self] in - self?.didFinish() + guard !isCancelled else { + didFinish() + return } - - for operation in operations { - finishOperation.addDependency(operation) - } - - var operationsWithFinish = operations - operationsWithFinish.append(finishOperation) - - operationQueue.addOperations(operationsWithFinish, waitUntilFinished: false) + operationQueue.isSuspended = false + } + + func addAnotherOperation(_ operation: Operation) { + guard !isCancelled else { return } + finishOperation?.addDependency(operation) + operationQueue.addOperation(operation) } override func cancel() { diff --git a/Frameworks/Account/Feedly/FeedlyOperation.swift b/Frameworks/Account/Feedly/FeedlyOperation.swift index abbaba525..cab7eb0e5 100644 --- a/Frameworks/Account/Feedly/FeedlyOperation.swift +++ b/Frameworks/Account/Feedly/FeedlyOperation.swift @@ -19,11 +19,13 @@ class FeedlyOperation: Operation { weak var delegate: FeedlyOperationDelegate? func didFinish() { + assert(Thread.isMainThread) self.isExecutingOperation = false self.isFinishedOperation = true } func didFinish(_ error: Error) { + assert(Thread.isMainThread) delegate?.feedlyOperation(self, didFailWith: error) didFinish() } diff --git a/Frameworks/Account/Feedly/Refresh/FeedlyGetStreamOperation.swift b/Frameworks/Account/Feedly/Refresh/FeedlyGetStreamOperation.swift index 0c2dbb076..9453d2364 100644 --- a/Frameworks/Account/Feedly/Refresh/FeedlyGetStreamOperation.swift +++ b/Frameworks/Account/Feedly/Refresh/FeedlyGetStreamOperation.swift @@ -15,6 +15,10 @@ protocol FeedlyEntryProviding: class { var parsedEntries: Set { get } } +protocol FeedlyGetStreamOperationDelegate: class { + func feedlyGetStreamOperation(_ operation: FeedlyGetStreamOperation, didGet stream: FeedlyStream) +} + /// Single responsibility is to get the stream content of a Collection from Feedly. final class FeedlyGetStreamOperation: FeedlyOperation, FeedlyEntryProviding { @@ -30,7 +34,8 @@ final class FeedlyGetStreamOperation: FeedlyOperation, FeedlyEntryProviding { var entries: [FeedlyEntry] { guard let entries = storedStream?.items else { - assertionFailure("Has a prior operation finished too early? Is the operation included in \(self.dependencies)?") + assert(isFinished, "This should only be called when the operation finishes without error.") + assertionFailure("Has this operation been addeded as a dependency on the caller?") return [] } return entries @@ -55,16 +60,19 @@ final class FeedlyGetStreamOperation: FeedlyOperation, FeedlyEntryProviding { private var storedParsedEntries: Set? - let account: Account let caller: FeedlyAPICaller let unreadOnly: Bool? let newerThan: Date? + let continuation: String? - init(account: Account, resource: FeedlyResourceId, caller: FeedlyAPICaller, newerThan: Date?, unreadOnly: Bool? = nil) { + weak var streamDelegate: FeedlyGetStreamOperationDelegate? + + init(account: Account, resource: FeedlyResourceId, caller: FeedlyAPICaller, continuation: String? = nil, newerThan: Date?, unreadOnly: Bool? = nil) { self.account = account self.resourceProvider = ResourceProvider(resource: resource) self.caller = caller + self.continuation = continuation self.unreadOnly = unreadOnly self.newerThan = newerThan } @@ -79,11 +87,15 @@ final class FeedlyGetStreamOperation: FeedlyOperation, FeedlyEntryProviding { return } - caller.getStream(for: resourceProvider.resource, newerThan: newerThan, unreadOnly: unreadOnly) { result in + caller.getStream(for: resourceProvider.resource, continuation: continuation, newerThan: newerThan, unreadOnly: unreadOnly) { result in switch result { case .success(let stream): self.storedStream = stream + + self.streamDelegate?.feedlyGetStreamOperation(self, didGet: stream) + self.didFinish() + case .failure(let error): self.didFinish(error) } diff --git a/Frameworks/Account/Feedly/Refresh/FeedlyOrganiseParsedItemsByFeedOperation.swift b/Frameworks/Account/Feedly/Refresh/FeedlyOrganiseParsedItemsByFeedOperation.swift index aa2e692c1..95fe8d37a 100644 --- a/Frameworks/Account/Feedly/Refresh/FeedlyOrganiseParsedItemsByFeedOperation.swift +++ b/Frameworks/Account/Feedly/Refresh/FeedlyOrganiseParsedItemsByFeedOperation.swift @@ -12,8 +12,7 @@ import os.log protocol FeedlyParsedItemsByFeedProviding { var providerName: String { get } - var allFeeds: Set { get } - func parsedItems(for feed: Feed) -> Set? + var parsedItemsKeyedByFeedId: [String: Set] { get } } /// Single responsibility is to group articles by their feeds. @@ -22,15 +21,9 @@ final class FeedlyOrganiseParsedItemsByFeedOperation: FeedlyOperation, FeedlyPar private let entryProvider: FeedlyEntryProviding private let log: OSLog - var allFeeds: Set { + var parsedItemsKeyedByFeedId: [String : Set] { assert(Thread.isMainThread) // Needs to be on main thread because Feed is a main-thread-only model type. - let keys = Set(itemsKeyedByFeedId.keys) - return account.flattenedFeeds().filter { keys.contains($0.feedID) } - } - - func parsedItems(for feed: Feed) -> Set? { - assert(Thread.isMainThread) // Needs to be on main thread because Feed is a main-thread-only model type. - return itemsKeyedByFeedId[feed.feedID] + return itemsKeyedByFeedId } var providerName: String { diff --git a/Frameworks/Account/Feedly/Refresh/FeedlySyncStarredArticlesOperation.swift b/Frameworks/Account/Feedly/Refresh/FeedlySyncStarredArticlesOperation.swift index 550ef5073..0c2f60bc9 100644 --- a/Frameworks/Account/Feedly/Refresh/FeedlySyncStarredArticlesOperation.swift +++ b/Frameworks/Account/Feedly/Refresh/FeedlySyncStarredArticlesOperation.swift @@ -8,18 +8,86 @@ import Foundation import os.log +import RSParser -final class FeedlySyncStarredArticlesOperation: FeedlyOperation { +final class FeedlySyncStarredArticlesOperation: FeedlyOperation, FeedlyOperationDelegate, FeedlyGetStreamOperationDelegate { private let account: Account private let operationQueue: OperationQueue private let caller: FeedlyAPICaller private let log: OSLog - init(account: Account, caller: FeedlyAPICaller, log: OSLog) { + private let setStatuses: FeedlySetStarredArticlesOperation + + /// Buffers every starred/saved entry from every page. + private class StarredEntryProvider: FeedlyEntryProviding { + var resource: FeedlyResourceId + + private(set) var parsedEntries = Set() + private(set) var entries = [FeedlyEntry]() + + init(resource: FeedlyResourceId) { + self.resource = resource + } + + func addEntries(from provider: FeedlyEntryProviding) { + entries.append(contentsOf: provider.entries) + parsedEntries.formUnion(provider.parsedEntries) + } + } + + private let entryProvider: StarredEntryProvider + + init(account: Account, credentials: Credentials, caller: FeedlyAPICaller, log: OSLog) { self.account = account self.caller = caller self.operationQueue = OperationQueue() + self.operationQueue.isSuspended = true self.log = log + + let saved = FeedlyTagResourceId.saved(for: credentials.username) + let provider = StarredEntryProvider(resource: saved) + self.entryProvider = provider + self.setStatuses = FeedlySetStarredArticlesOperation(account: account, + allStarredEntriesProvider: provider, + log: log) + + super.init() + + let getFirstPage = FeedlyGetStreamOperation(account: account, + resource: saved, + caller: caller, + newerThan: nil) + + let organiseByFeed = FeedlyOrganiseParsedItemsByFeedOperation(account: account, + entryProvider: provider, + log: log) + + let updateAccount = FeedlyUpdateAccountFeedsWithItemsOperation(account: account, + organisedItemsProvider: organiseByFeed, + log: log) + + getFirstPage.delegate = self + getFirstPage.streamDelegate = self + + setStatuses.addDependency(getFirstPage) + setStatuses.delegate = self + + organiseByFeed.addDependency(setStatuses) + organiseByFeed.delegate = self + + updateAccount.addDependency(organiseByFeed) + updateAccount.delegate = self + + let finishOperation = BlockOperation { [weak self] in + DispatchQueue.main.async { + self?.didFinish() + } + } + + finishOperation.addDependency(updateAccount) + + let operations = [getFirstPage, setStatuses, organiseByFeed, updateAccount, finishOperation] + operationQueue.addOperations(operations, waitUntilFinished: false) } override func cancel() { @@ -33,73 +101,31 @@ final class FeedlySyncStarredArticlesOperation: FeedlyOperation { return } - guard let user = caller.credentials?.username else { - didFinish(FeedlyAccountDelegateError.notLoggedIn) + operationQueue.isSuspended = false + } + + func feedlyGetStreamOperation(_ operation: FeedlyGetStreamOperation, didGet stream: FeedlyStream) { + entryProvider.addEntries(from: operation) + os_log(.debug, log: log, "Collecting %i items from %@", stream.items.count, stream.id) + + guard let continuation = stream.continuation else { return } - class Delegate: FeedlyOperationDelegate { - var error: Error? - weak var compoundOperation: FeedlyCompoundOperation? - - func feedlyOperation(_ operation: FeedlyOperation, didFailWith error: Error) { - compoundOperation?.cancel() - self.error = error - } - } + let nextPageOperation = FeedlyGetStreamOperation(account: operation.account, + resource: operation.resource, + caller: operation.caller, + continuation: continuation, + newerThan: operation.newerThan) + nextPageOperation.delegate = self + nextPageOperation.streamDelegate = self - let delegate = Delegate() - - let syncSaved = FeedlyCompoundOperation { - - let saved = FeedlyTagResourceId.saved(for: user) - os_log(.debug, log: log, "Getting starred articles from \"%@\".", saved.id) - - let getSavedStream = FeedlyGetStreamOperation(account: account, - resource: saved, - caller: caller, - newerThan: nil) - getSavedStream.delegate = delegate - - // set statuses - let setStatuses = FeedlySetStarredArticlesOperation(account: account, - allStarredEntriesProvider: getSavedStream, - log: log) - setStatuses.delegate = delegate - setStatuses.addDependency(getSavedStream) - - // ingest articles - let organiseByFeed = FeedlyOrganiseParsedItemsByFeedOperation(account: account, - entryProvider: getSavedStream, - log: log) - organiseByFeed.delegate = delegate - organiseByFeed.addDependency(setStatuses) - - let updateAccount = FeedlyUpdateAccountFeedsWithItemsOperation(account: account, - organisedItemsProvider: organiseByFeed, - log: log) - updateAccount.delegate = delegate - updateAccount.addDependency(organiseByFeed) - - return [getSavedStream, setStatuses, organiseByFeed, updateAccount] - } - - delegate.compoundOperation = syncSaved - - let finalOperation = BlockOperation { [weak self] in - guard let self = self else { - return - } - if let error = delegate.error { - self.didFinish(error) - } else { - self.didFinish() - } - os_log(.debug, log: self.log, "Done syncing starred articles.") - } - - finalOperation.addDependency(syncSaved) - operationQueue.addOperations([syncSaved, finalOperation], waitUntilFinished: false) + setStatuses.addDependency(nextPageOperation) + operationQueue.addOperation(nextPageOperation) } + func feedlyOperation(_ operation: FeedlyOperation, didFailWith error: Error) { + operationQueue.cancelAllOperations() + didFinish(error) + } } diff --git a/Frameworks/Account/Feedly/Refresh/FeedlySyncStrategy.swift b/Frameworks/Account/Feedly/Refresh/FeedlySyncStrategy.swift index 757f20466..f4571813c 100644 --- a/Frameworks/Account/Feedly/Refresh/FeedlySyncStrategy.swift +++ b/Frameworks/Account/Feedly/Refresh/FeedlySyncStrategy.swift @@ -49,6 +49,11 @@ final class FeedlySyncStrategy { return } + guard let credentials = caller.credentials else { + completionHandler(.failure(FeedlyAccountDelegateError.notLoggedIn)) + return + } + let sendArticleStatuses = FeedlySendArticleStatusesOperation(database: database, caller: caller, log: log) sendArticleStatuses.delegate = self @@ -85,7 +90,7 @@ final class FeedlySyncStrategy { getCollectionStreams.queueDelegate = self getCollectionStreams.addDependency(getCollections) - let syncStarred = FeedlySyncStarredArticlesOperation(account: account, caller: caller, log: log) + let syncStarred = FeedlySyncStarredArticlesOperation(account: account, credentials: credentials, caller: caller, log: log) syncStarred.addDependency(getCollections) syncStarred.addDependency(mirrorCollectionsAsFolders) syncStarred.addDependency(createFeedsOperation) diff --git a/Frameworks/Account/Feedly/Refresh/FeedlyUpdateAccountFeedsWithItemsOperation.swift b/Frameworks/Account/Feedly/Refresh/FeedlyUpdateAccountFeedsWithItemsOperation.swift index f272f84e2..e6620a61a 100644 --- a/Frameworks/Account/Feedly/Refresh/FeedlyUpdateAccountFeedsWithItemsOperation.swift +++ b/Frameworks/Account/Feedly/Refresh/FeedlyUpdateAccountFeedsWithItemsOperation.swift @@ -29,19 +29,10 @@ final class FeedlyUpdateAccountFeedsWithItemsOperation: FeedlyOperation { return } - let allFeeds = organisedItemsProvider.allFeeds + let feedIDsAndItems = organisedItemsProvider.parsedItemsKeyedByFeedId - os_log(.debug, log: log, "Begin updating %i feeds for \"%@\"", allFeeds.count, organisedItemsProvider.providerName) - - var feedIDsAndItems = [String: Set]() - for feed in allFeeds { - guard let items = organisedItemsProvider.parsedItems(for: feed) else { - continue - } - feedIDsAndItems[feed.feedID] = items - } account.update(feedIDsAndItems: feedIDsAndItems, defaultRead: true) { - os_log(.debug, log: self.log, "Finished updating feeds for \"%@\"", self.organisedItemsProvider.providerName) + os_log(.debug, log: self.log, "Updated %i feeds for \"%@\"", feedIDsAndItems.count, self.organisedItemsProvider.providerName) self.didFinish() } }