From 6181f416a4d1e5f1ba268399e91acde143581ae3 Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Sun, 19 Jan 2020 14:19:06 -0800 Subject: [PATCH] =?UTF-8?q?Revise=20FeedlyOperation=20to=20work=20with=20M?= =?UTF-8?q?ainThreadOperation=20properly.=20We=E2=80=99re=20still=20using?= =?UTF-8?q?=20inheritance=20=E2=80=94=C2=A0FeedlyOperation=20is=20a=20base?= =?UTF-8?q?=20class.=20I=20tried=20and=20failed=20to=20come=20up=20with=20?= =?UTF-8?q?a=20better=20solution.=20Everything=20other=20solution=20result?= =?UTF-8?q?ed=20in=20a=20lot=20of=20boilerplate=20code=20being=20replicate?= =?UTF-8?q?d.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../Account/Account.xcodeproj/project.pbxproj | 4 +- .../FeedlyAddExistingFeedOperation.swift | 25 ++++----- .../FeedlyAddFeedToCollectionOperation.swift | 16 +++--- .../FeedlyAddNewFeedOperation.swift | 34 +++++------- .../FeedlyCheckpointOperation.swift | 5 +- ...teFeedsForCollectionFoldersOperation.swift | 3 +- .../FeedlyDownloadArticlesOperation.swift | 27 ++++----- ...yFetchIdsForMissingArticlesOperation.swift | 4 +- .../FeedlyGetCollectionsOperation.swift | 8 +-- .../FeedlyGetEntriesOperation.swift | 9 ++- .../FeedlyGetStreamContentsOperation.swift | 10 ++-- .../FeedlyGetStreamIdsOperation.swift | 7 +-- .../FeedlyGetUpdatedArticleIdsOperation.swift | 4 +- ...edlyIngestStarredArticleIdsOperation.swift | 12 ++-- ...eedlyIngestStreamArticleIdsOperation.swift | 8 +-- ...eedlyIngestUnreadArticleIdsOperation.swift | 12 ++-- .../Operations/FeedlyLogoutOperation.swift | 4 +- ...yMirrorCollectionsAsFoldersOperation.swift | 5 +- .../Feedly/Operations/FeedlyOperation.swift | 55 +++++++------------ ...lyOrganiseParsedItemsByFeedOperation.swift | 6 +- .../FeedlyRefreshAccessTokenOperation.swift | 8 +-- .../FeedlyRequestStreamsOperation.swift | 5 +- .../Operations/FeedlySearchOperation.swift | 10 ++-- .../FeedlySendArticleStatusesOperation.swift | 7 ++- .../Operations/FeedlySyncAllOperation.swift | 52 ++++++++---------- .../FeedlySyncStreamContentsOperation.swift | 33 +++++------ ...UpdateAccountFeedsWithItemsOperation.swift | 8 +-- 27 files changed, 166 insertions(+), 215 deletions(-) diff --git a/Frameworks/Account/Account.xcodeproj/project.pbxproj b/Frameworks/Account/Account.xcodeproj/project.pbxproj index 918ab69f1..e808b6d14 100644 --- a/Frameworks/Account/Account.xcodeproj/project.pbxproj +++ b/Frameworks/Account/Account.xcodeproj/project.pbxproj @@ -399,7 +399,6 @@ 9EF1B10623590D61000A486A /* FeedlyGetStreamIdsOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyGetStreamIdsOperation.swift; sourceTree = ""; }; 9EF1B10823590E93000A486A /* FeedlyStreamIds.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyStreamIds.swift; sourceTree = ""; }; 9EF2602B23C91FFE006D160C /* FeedlyGetUpdatedArticleIdsOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyGetUpdatedArticleIdsOperation.swift; sourceTree = ""; }; - 9EF35F79234E830E003AE2AE /* FeedlyCompoundOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyCompoundOperation.swift; sourceTree = ""; }; D511EEB5202422BB00712EC3 /* Account_project_debug.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = Account_project_debug.xcconfig; sourceTree = ""; }; D511EEB6202422BB00712EC3 /* Account_target.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = Account_target.xcconfig; sourceTree = ""; }; D511EEB7202422BB00712EC3 /* Account_project_release.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = Account_project_release.xcconfig; sourceTree = ""; }; @@ -700,7 +699,7 @@ isa = PBXGroup; children = ( 9E1D1554233431A600F4944C /* FeedlyOperation.swift */, - 9EF35F79234E830E003AE2AE /* FeedlyCompoundOperation.swift */, + 9E1D154C233370D800F4944C /* FeedlySyncAllOperation.swift */, 9EA643D2239305680018A28C /* FeedlySearchOperation.swift */, 9E7299D623505E9600DAEFB7 /* FeedlyAddFeedToCollectionOperation.swift */, 9EB1D575238E6A3900A753D7 /* FeedlyAddNewFeedOperation.swift */, @@ -721,7 +720,6 @@ 9EEEF7202355277F009E9D80 /* FeedlyIngestStarredArticleIdsOperation.swift */, 9E84DC462359A23200D6E809 /* FeedlyIngestUnreadArticleIdsOperation.swift */, 9EF2602B23C91FFE006D160C /* FeedlyGetUpdatedArticleIdsOperation.swift */, - 9E1D154C233370D800F4944C /* FeedlySyncAllOperation.swift */, 9E672393236F7CA0000BE141 /* FeedlyRefreshAccessTokenOperation.swift */, 9E784EBD237E890600099B1B /* FeedlyLogoutOperation.swift */, 9E5DE60D23C3F4B70064DA30 /* FeedlyFetchIdsForMissingArticlesOperation.swift */, diff --git a/Frameworks/Account/Feedly/Operations/FeedlyAddExistingFeedOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyAddExistingFeedOperation.swift index 1b921b28d..d79367e24 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyAddExistingFeedOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyAddExistingFeedOperation.swift @@ -12,18 +12,17 @@ import RSWeb import RSCore class FeedlyAddExistingFeedOperation: FeedlyOperation, FeedlyOperationDelegate, FeedlyCheckpointOperationDelegate { - private let operationQueue: MainThreadOperationQueue - + + private let operationQueue = MainThreadOperationQueue() var addCompletionHandler: ((Result) -> ())? - + init(account: Account, credentials: Credentials, resource: FeedlyFeedResourceId, service: FeedlyAddFeedToCollectionService, container: Container, progress: DownloadProgress, log: OSLog) throws { let validator = FeedlyFeedContainerValidator(container: container) let (folder, collectionId) = try validator.getValidContainer() - self.operationQueue = MainThreadOperationQueue() self.operationQueue.suspend() - + super.init() self.downloadProgress = progress @@ -35,26 +34,24 @@ class FeedlyAddExistingFeedOperation: FeedlyOperation, FeedlyOperationDelegate, let createFeeds = FeedlyCreateFeedsForCollectionFoldersOperation(account: account, feedsAndFoldersProvider: addRequest, log: log) createFeeds.downloadProgress = progress - self.operationQueue.make(createFeeds, dependOn: addRequest) + createFeeds.addDependency(addRequest) self.operationQueue.addOperation(createFeeds) let finishOperation = FeedlyCheckpointOperation() finishOperation.checkpointDelegate = self finishOperation.downloadProgress = progress - self.operationQueue.make(finishOperation, dependOn: createFeeds) + finishOperation.addDependency(createFeeds) self.operationQueue.addOperation(finishOperation) } - override func cancel() { - operationQueue.cancelAllOperations() - super.cancel() - didFinish() - } - override func run() { - super.run() operationQueue.resume() } + + override func didCancel() { + operationQueue.cancelAllOperations() + addCompletionHandler = nil + } func feedlyOperation(_ operation: FeedlyOperation, didFailWith error: Error) { addCompletionHandler?(.failure(error)) diff --git a/Frameworks/Account/Feedly/Operations/FeedlyAddFeedToCollectionOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyAddFeedToCollectionOperation.swift index 12252b8ee..ce749a83d 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyAddFeedToCollectionOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyAddFeedToCollectionOperation.swift @@ -13,13 +13,14 @@ protocol FeedlyAddFeedToCollectionService { } final class FeedlyAddFeedToCollectionOperation: FeedlyOperation, FeedlyFeedsAndFoldersProviding, FeedlyResourceProviding { + let feedName: String? let collectionId: String let service: FeedlyAddFeedToCollectionService let account: Account let folder: Folder let feedResource: FeedlyFeedResourceId - + init(account: Account, folder: Folder, feedResource: FeedlyFeedResourceId, feedName: String? = nil, collectionId: String, service: FeedlyAddFeedToCollectionService) { self.account = account self.folder = folder @@ -36,8 +37,6 @@ final class FeedlyAddFeedToCollectionOperation: FeedlyOperation, FeedlyFeedsAndF } override func run() { - super.run() - service.addFeed(with: feedResource, title: feedName, toCollectionWith: collectionId) { [weak self] result in guard let self = self else { return @@ -49,8 +48,11 @@ final class FeedlyAddFeedToCollectionOperation: FeedlyOperation, FeedlyFeedsAndF self.didCompleteRequest(result) } } - - private func didCompleteRequest(_ result: Result<[FeedlyFeed], Error>) { +} + +private extension FeedlyAddFeedToCollectionOperation { + + func didCompleteRequest(_ result: Result<[FeedlyFeed], Error>) { switch result { case .success(let feedlyFeeds): feedsAndFolders = [(feedlyFeeds, folder)] @@ -58,13 +60,13 @@ final class FeedlyAddFeedToCollectionOperation: FeedlyOperation, FeedlyFeedsAndF let feedsWithCreatedFeedId = feedlyFeeds.filter { $0.id == resource.id } if feedsWithCreatedFeedId.isEmpty { - didFinish(AccountError.createErrorNotFound) + didFinish(with: AccountError.createErrorNotFound) } else { didFinish() } case .failure(let error): - didFinish(error) + didFinish(with: error) } } } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyAddNewFeedOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyAddNewFeedOperation.swift index 56fd11e0f..7cf8cb7c2 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyAddNewFeedOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyAddNewFeedOperation.swift @@ -13,7 +13,8 @@ import RSWeb import RSCore class FeedlyAddNewFeedOperation: FeedlyOperation, FeedlyOperationDelegate, FeedlySearchOperationDelegate, FeedlyCheckpointOperationDelegate { - private let operationQueue: MainThreadOperationQueue + + private let operationQueue = MainThreadOperationQueue() private let folder: Folder private let collectionId: String private let url: String @@ -25,16 +26,16 @@ class FeedlyAddNewFeedOperation: FeedlyOperation, FeedlyOperationDelegate, Feedl private let syncUnreadIdsService: FeedlyGetStreamIdsService private let getStreamContentsService: FeedlyGetStreamContentsService private let log: OSLog - + private var feedResourceId: FeedlyFeedResourceId? var addCompletionHandler: ((Result) -> ())? 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) (self.folder, self.collectionId) = try validator.getValidContainer() self.url = url - self.operationQueue = MainThreadOperationQueue() self.operationQueue.suspend() self.account = account self.credentials = credentials @@ -44,9 +45,9 @@ class FeedlyAddNewFeedOperation: FeedlyOperation, FeedlyOperationDelegate, Feedl self.syncUnreadIdsService = syncUnreadIdsService self.getStreamContentsService = getStreamContentsService self.log = log - + super.init() - + self.downloadProgress = progress let search = FeedlySearchOperation(query: url, locale: .current, service: searchService) @@ -57,24 +58,20 @@ class FeedlyAddNewFeedOperation: FeedlyOperation, FeedlyOperationDelegate, Feedl } override func run() { - super.run() operationQueue.resume() } - override func cancel() { - super.cancel() + override func didCancel() { operationQueue.cancelAllOperations() addCompletionHandler = nil } - private var feedResourceId: FeedlyFeedResourceId? - func feedlySearchOperation(_ operation: FeedlySearchOperation, didGet response: FeedlyFeedsSearchResponse) { guard !isCanceled else { return } guard let first = response.results.first else { - return didFinish(AccountError.createErrorNotFound) + return didFinish(with: AccountError.createErrorNotFound) } let feedResourceId = FeedlyFeedResourceId(id: first.feedId) @@ -86,24 +83,24 @@ class FeedlyAddNewFeedOperation: FeedlyOperation, FeedlyOperationDelegate, Feedl operationQueue.addOperation(addRequest) let createFeeds = FeedlyCreateFeedsForCollectionFoldersOperation(account: account, feedsAndFoldersProvider: addRequest, log: log) - operationQueue.make(createFeeds, dependOn: addRequest) + createFeeds.addDependency(addRequest) createFeeds.downloadProgress = downloadProgress operationQueue.addOperation(createFeeds) let syncUnread = FeedlyIngestUnreadArticleIdsOperation(account: account, credentials: credentials, service: syncUnreadIdsService, database: database, newerThan: nil, log: log) - operationQueue.make(syncUnread, dependOn: createFeeds) + syncUnread.addDependency(createFeeds) syncUnread.downloadProgress = downloadProgress operationQueue.addOperation(syncUnread) let syncFeed = FeedlySyncStreamContentsOperation(account: account, resource: feedResourceId, service: getStreamContentsService, isPagingEnabled: false, newerThan: nil, log: log) - operationQueue.make(syncFeed, dependOn: syncUnread) + syncFeed.addDependency(syncUnread) syncFeed.downloadProgress = downloadProgress operationQueue.addOperation(syncFeed) let finishOperation = FeedlyCheckpointOperation() finishOperation.checkpointDelegate = self finishOperation.downloadProgress = downloadProgress - operationQueue.make(finishOperation, dependOn: syncFeed) + finishOperation.addDependency(syncFeed) operationQueue.addOperation(finishOperation) } @@ -118,7 +115,6 @@ class FeedlyAddNewFeedOperation: FeedlyOperation, FeedlyOperationDelegate, Feedl guard !isCanceled else { return } - defer { didFinish() } @@ -126,14 +122,12 @@ class FeedlyAddNewFeedOperation: FeedlyOperation, FeedlyOperationDelegate, Feedl guard let handler = addCompletionHandler else { return } - if let feedResource = feedResourceId, let feed = folder.existingWebFeed(withWebFeedID: feedResource.id) { handler(.success(feed)) - - } else { + } + else { handler(.failure(AccountError.createErrorNotFound)) } - addCompletionHandler = nil } } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyCheckpointOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyCheckpointOperation.swift index 85c3b5733..d577abeae 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyCheckpointOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyCheckpointOperation.swift @@ -12,13 +12,12 @@ protocol FeedlyCheckpointOperationDelegate: class { func feedlyCheckpointOperationDidReachCheckpoint(_ operation: FeedlyCheckpointOperation) } -/// Single responsibility is to let the delegate know an instance is executing. The semantics are up to the delegate. +/// Let the delegate know an instance is executing. The semantics are up to the delegate. final class FeedlyCheckpointOperation: FeedlyOperation { weak var checkpointDelegate: FeedlyCheckpointOperationDelegate? - + override func run() { - super.run() defer { didFinish() } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyCreateFeedsForCollectionFoldersOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyCreateFeedsForCollectionFoldersOperation.swift index 98895f204..9d260a7ad 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyCreateFeedsForCollectionFoldersOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyCreateFeedsForCollectionFoldersOperation.swift @@ -15,7 +15,7 @@ final class FeedlyCreateFeedsForCollectionFoldersOperation: FeedlyOperation { let account: Account let feedsAndFoldersProvider: FeedlyFeedsAndFoldersProviding let log: OSLog - + init(account: Account, feedsAndFoldersProvider: FeedlyFeedsAndFoldersProviding, log: OSLog) { self.feedsAndFoldersProvider = feedsAndFoldersProvider self.account = account @@ -23,7 +23,6 @@ final class FeedlyCreateFeedsForCollectionFoldersOperation: FeedlyOperation { } override func run() { - super.run() defer { didFinish() } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyDownloadArticlesOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyDownloadArticlesOperation.swift index cbbc466b7..49abcaee3 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyDownloadArticlesOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyDownloadArticlesOperation.swift @@ -9,8 +9,10 @@ import Foundation import os.log import RSCore +import RSWeb class FeedlyDownloadArticlesOperation: FeedlyOperation { + private let account: Account private let log: OSLog private let missingArticleEntryIdProvider: FeedlyEntryIdentifierProviding @@ -27,24 +29,12 @@ class FeedlyDownloadArticlesOperation: FeedlyOperation { self.getEntriesService = getEntriesService self.finishOperation = FeedlyCheckpointOperation() self.log = log - super.init() - self.finishOperation.checkpointDelegate = self self.operationQueue.addOperation(self.finishOperation) } - override func cancel() { - // TODO: fix error on below line: "Expression type '()' is ambiguous without more context" - //os_log(.debug, log: log, "Cancelling %{public}@.", self) - operationQueue.cancelAllOperations() - super.cancel() - didFinish() - } - override func run() { - super.run() - var articleIds = missingArticleEntryIdProvider.entryIds articleIds.formUnion(updatedArticleEntryIdProvider.entryIds) @@ -62,7 +52,7 @@ class FeedlyDownloadArticlesOperation: FeedlyOperation { parsedItemProvider: getEntries, log: log) organiseByFeed.delegate = self - self.operationQueue.make(organiseByFeed, dependOn: getEntries) + organiseByFeed.addDependency(getEntries) self.operationQueue.addOperation(organiseByFeed) let updateAccount = FeedlyUpdateAccountFeedsWithItemsOperation(account: account, @@ -70,14 +60,21 @@ class FeedlyDownloadArticlesOperation: FeedlyOperation { log: log) updateAccount.delegate = self - self.operationQueue.make(updateAccount, dependOn: organiseByFeed) + updateAccount.addDependency(organiseByFeed) self.operationQueue.addOperation(updateAccount) - self.operationQueue.make(finishOperation, dependOn: updateAccount) + finishOperation.addDependency(updateAccount) } operationQueue.resume() } + + override func didCancel() { + // TODO: fix error on below line: "Expression type '()' is ambiguous without more context" + //os_log(.debug, log: log, "Cancelling %{public}@.", self) + operationQueue.cancelAllOperations() + didFinish() + } } extension FeedlyDownloadArticlesOperation: FeedlyCheckpointOperationDelegate { diff --git a/Frameworks/Account/Feedly/Operations/FeedlyFetchIdsForMissingArticlesOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyFetchIdsForMissingArticlesOperation.swift index 4fb05e27f..bc37dfdb7 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyFetchIdsForMissingArticlesOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyFetchIdsForMissingArticlesOperation.swift @@ -10,6 +10,7 @@ import Foundation import os.log final class FeedlyFetchIdsForMissingArticlesOperation: FeedlyOperation, FeedlyEntryIdentifierProviding { + private let account: Account private let log: OSLog @@ -21,7 +22,6 @@ final class FeedlyFetchIdsForMissingArticlesOperation: FeedlyOperation, FeedlyEn } override func run() { - super.run() account.fetchArticleIDsForStatusesWithoutArticlesNewerThanCutoffDate { result in switch result { case .success(let articleIds): @@ -29,7 +29,7 @@ final class FeedlyFetchIdsForMissingArticlesOperation: FeedlyOperation, FeedlyEn self.didFinish() case .failure(let error): - self.didFinish(error) + self.didFinish(with: error) } } } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyGetCollectionsOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyGetCollectionsOperation.swift index 34075344f..297bd3cd7 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyGetCollectionsOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyGetCollectionsOperation.swift @@ -13,22 +13,20 @@ protocol FeedlyCollectionProviding: class { var collections: [FeedlyCollection] { get } } -/// Single responsibility is to get Collections from Feedly. +/// Get Collections from Feedly. final class FeedlyGetCollectionsOperation: FeedlyOperation, FeedlyCollectionProviding { let service: FeedlyGetCollectionsService let log: OSLog private(set) var collections = [FeedlyCollection]() - + init(service: FeedlyGetCollectionsService, log: OSLog) { self.service = service self.log = log } override func run() { - super.run() - os_log(.debug, log: log, "Requesting collections.") service.getCollections { result in @@ -40,7 +38,7 @@ final class FeedlyGetCollectionsOperation: FeedlyOperation, FeedlyCollectionProv case .failure(let error): os_log(.debug, log: self.log, "Unable to request collections: %{public}@.", error as NSError) - self.didFinish(error) + self.didFinish(with: error) } } } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyGetEntriesOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyGetEntriesOperation.swift index 3293a93b4..f306792f6 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyGetEntriesOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyGetEntriesOperation.swift @@ -10,13 +10,14 @@ import Foundation import os.log import RSParser -/// Single responsibility is to get full entries for the entry identifiers. +/// Get full entries for the entry identifiers. final class FeedlyGetEntriesOperation: FeedlyOperation, FeedlyEntryProviding, FeedlyParsedItemProviding { + let account: Account let service: FeedlyGetEntriesService let provider: FeedlyEntryIdentifierProviding let log: OSLog - + init(account: Account, service: FeedlyGetEntriesService, provider: FeedlyEntryIdentifierProviding, log: OSLog) { self.account = account self.service = service @@ -55,8 +56,6 @@ final class FeedlyGetEntriesOperation: FeedlyOperation, FeedlyEntryProviding, Fe } override func run() { - super.run() - service.getEntries(for: provider.entryIds) { result in switch result { case .success(let entries): @@ -65,7 +64,7 @@ final class FeedlyGetEntriesOperation: FeedlyOperation, FeedlyEntryProviding, Fe case .failure(let error): os_log(.debug, log: self.log, "Unable to get entries: %{public}@.", error as NSError) - self.didFinish(error) + self.didFinish(with: error) } } } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyGetStreamContentsOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyGetStreamContentsOperation.swift index 7cfafa827..4c67bcfd4 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyGetStreamContentsOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyGetStreamContentsOperation.swift @@ -23,7 +23,7 @@ protocol FeedlyGetStreamContentsOperationDelegate: class { func feedlyGetStreamContentsOperation(_ operation: FeedlyGetStreamContentsOperation, didGetContentsOf stream: FeedlyStream) } -/// Single responsibility is to get the stream content of a Collection from Feedly. +/// Get the stream content of a Collection from Feedly. final class FeedlyGetStreamContentsOperation: FeedlyOperation, FeedlyEntryProviding, FeedlyParsedItemProviding { struct ResourceProvider: FeedlyResourceProviding { @@ -38,7 +38,7 @@ final class FeedlyGetStreamContentsOperation: FeedlyOperation, FeedlyEntryProvid var entries: [FeedlyEntry] { guard let entries = stream?.items else { - assert(isFinished, "This should only be called when the operation finishes without error.") +// 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 [] } @@ -82,7 +82,7 @@ final class FeedlyGetStreamContentsOperation: FeedlyOperation, FeedlyEntryProvid let log: OSLog weak var streamDelegate: FeedlyGetStreamContentsOperationDelegate? - + init(account: Account, resource: FeedlyResourceId, service: FeedlyGetStreamContentsService, continuation: String? = nil, newerThan: Date?, unreadOnly: Bool? = nil, log: OSLog) { self.account = account self.resourceProvider = ResourceProvider(resource: resource) @@ -98,8 +98,6 @@ final class FeedlyGetStreamContentsOperation: FeedlyOperation, FeedlyEntryProvid } override func run() { - super.run() - service.getStreamContents(for: resourceProvider.resource, continuation: continuation, newerThan: newerThan, unreadOnly: unreadOnly) { result in switch result { case .success(let stream): @@ -111,7 +109,7 @@ final class FeedlyGetStreamContentsOperation: FeedlyOperation, FeedlyEntryProvid case .failure(let error): os_log(.debug, log: self.log, "Unable to get stream contents: %{public}@.", error as NSError) - self.didFinish(error) + self.didFinish(with: error) } } } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyGetStreamIdsOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyGetStreamIdsOperation.swift index 0c25f4397..4f6fea202 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyGetStreamIdsOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyGetStreamIdsOperation.swift @@ -18,7 +18,6 @@ final class FeedlyGetStreamIdsOperation: FeedlyOperation, FeedlyEntryIdentifierP var entryIds: Set { guard let ids = streamIds?.ids else { - 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 [] } @@ -34,7 +33,7 @@ final class FeedlyGetStreamIdsOperation: FeedlyOperation, FeedlyEntryIdentifierP let unreadOnly: Bool? let newerThan: Date? let log: OSLog - + init(account: Account, resource: FeedlyResourceId, service: FeedlyGetStreamIdsService, continuation: String? = nil, newerThan: Date? = nil, unreadOnly: Bool?, log: OSLog) { self.account = account self.resource = resource @@ -48,8 +47,6 @@ final class FeedlyGetStreamIdsOperation: FeedlyOperation, FeedlyEntryIdentifierP weak var streamIdsDelegate: FeedlyGetStreamIdsOperationDelegate? override func run() { - super.run() - service.getStreamIds(for: resource, continuation: continuation, newerThan: newerThan, unreadOnly: unreadOnly) { result in switch result { case .success(let stream): @@ -61,7 +58,7 @@ final class FeedlyGetStreamIdsOperation: FeedlyOperation, FeedlyEntryIdentifierP case .failure(let error): os_log(.debug, log: self.log, "Unable to get stream ids: %{public}@.", error as NSError) - self.didFinish(error) + self.didFinish(with: error) } } } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyGetUpdatedArticleIdsOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyGetUpdatedArticleIdsOperation.swift index 79eacb835..1ed7a2c10 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyGetUpdatedArticleIdsOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyGetUpdatedArticleIdsOperation.swift @@ -14,6 +14,7 @@ import os.log /// Typically, it pages through the article ids of the global.all stream. /// When all the article ids are collected, it is the responsibility of another operation to download them when appropriate. class FeedlyGetUpdatedArticleIdsOperation: FeedlyOperation, FeedlyEntryIdentifierProviding { + private let account: Account private let resource: FeedlyResourceId private let service: FeedlyGetStreamIdsService @@ -40,7 +41,6 @@ class FeedlyGetUpdatedArticleIdsOperation: FeedlyOperation, FeedlyEntryIdentifie private var storedUpdatedArticleIds = Set() override func run() { - super.run() getStreamIds(nil) } @@ -73,7 +73,7 @@ class FeedlyGetUpdatedArticleIdsOperation: FeedlyOperation, FeedlyEntryIdentifie getStreamIds(continuation) case .failure(let error): - didFinish(error) + didFinish(with: error) } } } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyIngestStarredArticleIdsOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyIngestStarredArticleIdsOperation.swift index 2bd7f867b..02dc42043 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyIngestStarredArticleIdsOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyIngestStarredArticleIdsOperation.swift @@ -10,13 +10,14 @@ import Foundation import os.log import SyncDatabase -/// Single responsibility is to clone locally the remote starred article state. +/// Clone locally the remote starred article state. /// /// Typically, it pages through the article ids of the global.saved stream. /// When all the article ids are collected, a status is created for each. /// The article ids previously marked as starred but not collected become unstarred. /// So this operation has side effects *for the entire account* it operates on. final class FeedlyIngestStarredArticleIdsOperation: FeedlyOperation { + private let account: Account private let resource: FeedlyResourceId private let service: FeedlyGetStreamIdsService @@ -38,7 +39,6 @@ final class FeedlyIngestStarredArticleIdsOperation: FeedlyOperation { } override func run() { - super.run() getStreamIds(nil) } @@ -65,7 +65,7 @@ final class FeedlyIngestStarredArticleIdsOperation: FeedlyOperation { getStreamIds(continuation) case .failure(let error): - didFinish(error) + didFinish(with: error) } } @@ -84,7 +84,7 @@ final class FeedlyIngestStarredArticleIdsOperation: FeedlyOperation { self.updateStarredStatuses() case .failure(let error): - self.didFinish(error) + self.didFinish(with: error) } } } @@ -101,7 +101,7 @@ final class FeedlyIngestStarredArticleIdsOperation: FeedlyOperation { self.processStarredArticleIDs(localStarredArticleIDs) case .failure(let error): - self.didFinish(error) + self.didFinish(with: error) } } } @@ -142,7 +142,7 @@ final class FeedlyIngestStarredArticleIdsOperation: FeedlyOperation { self.didFinish() return } - self.didFinish(error) + self.didFinish(with: error) } } } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyIngestStreamArticleIdsOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyIngestStreamArticleIdsOperation.swift index adf36c3a5..12b906d12 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyIngestStreamArticleIdsOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyIngestStreamArticleIdsOperation.swift @@ -9,12 +9,13 @@ import Foundation import os.log -/// Single responsibility is to ensure a status exists for every article id the user might be interested in. +/// Ensure a status exists for every article id the user might be interested in. /// /// Typically, it pages through the article ids of the global.all stream. /// As the article ids are collected, a default read status is created for each. /// So this operation has side effects *for the entire account* it operates on. class FeedlyIngestStreamArticleIdsOperation: FeedlyOperation { + private let account: Account private let resource: FeedlyResourceId private let service: FeedlyGetStreamIdsService @@ -33,7 +34,6 @@ class FeedlyIngestStreamArticleIdsOperation: FeedlyOperation { } override func run() { - super.run() getStreamIds(nil) } @@ -52,7 +52,7 @@ class FeedlyIngestStreamArticleIdsOperation: FeedlyOperation { account.createStatusesIfNeeded(articleIDs: Set(streamIds.ids)) { databaseError in if let error = databaseError { - self.didFinish(error) + self.didFinish(with: error) return } @@ -65,7 +65,7 @@ class FeedlyIngestStreamArticleIdsOperation: FeedlyOperation { self.getStreamIds(continuation) } case .failure(let error): - didFinish(error) + didFinish(with: error) } } } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyIngestUnreadArticleIdsOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyIngestUnreadArticleIdsOperation.swift index a695cd8ee..669a9672d 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyIngestUnreadArticleIdsOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyIngestUnreadArticleIdsOperation.swift @@ -11,13 +11,14 @@ import os.log import RSParser import SyncDatabase -/// Single responsibility is to clone locally the remote unread article state. +/// Clone locally the remote unread article state. /// /// Typically, it pages through the unread article ids of the global.all stream. /// When all the unread article ids are collected, a status is created for each. /// The article ids previously marked as unread but not collected become read. /// So this operation has side effects *for the entire account* it operates on. final class FeedlyIngestUnreadArticleIdsOperation: FeedlyOperation { + private let account: Account private let resource: FeedlyResourceId private let service: FeedlyGetStreamIdsService @@ -39,7 +40,6 @@ final class FeedlyIngestUnreadArticleIdsOperation: FeedlyOperation { } override func run() { - super.run() getStreamIds(nil) } @@ -66,7 +66,7 @@ final class FeedlyIngestUnreadArticleIdsOperation: FeedlyOperation { getStreamIds(continuation) case .failure(let error): - didFinish(error) + didFinish(with: error) } } @@ -85,7 +85,7 @@ final class FeedlyIngestUnreadArticleIdsOperation: FeedlyOperation { self.updateUnreadStatuses() case .failure(let error): - self.didFinish(error) + self.didFinish(with: error) } } } @@ -102,7 +102,7 @@ final class FeedlyIngestUnreadArticleIdsOperation: FeedlyOperation { self.processUnreadArticleIDs(localUnreadArticleIDs) case .failure(let error): - self.didFinish(error) + self.didFinish(with: error) } } } @@ -142,7 +142,7 @@ final class FeedlyIngestUnreadArticleIdsOperation: FeedlyOperation { self.didFinish() return } - self.didFinish(error) + self.didFinish(with: error) } } } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyLogoutOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyLogoutOperation.swift index 6e83990db..29fb7eed6 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyLogoutOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyLogoutOperation.swift @@ -14,6 +14,7 @@ protocol FeedlyLogoutService { } final class FeedlyLogoutOperation: FeedlyOperation { + let service: FeedlyLogoutService let account: Account let log: OSLog @@ -25,7 +26,6 @@ final class FeedlyLogoutOperation: FeedlyOperation { } override func run() { - super.run() os_log("Requesting logout of %{public}@ account.", "\(account.type)") service.logout(completion: didCompleteLogout(_:)) } @@ -45,7 +45,7 @@ final class FeedlyLogoutOperation: FeedlyOperation { case .failure(let error): os_log("Logout failed because %{public}@.", error as NSError) - didFinish(error) + didFinish(with: error) } } } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyMirrorCollectionsAsFoldersOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyMirrorCollectionsAsFoldersOperation.swift index 3e1727431..084564b94 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyMirrorCollectionsAsFoldersOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyMirrorCollectionsAsFoldersOperation.swift @@ -17,7 +17,7 @@ protocol FeedlyFeedsAndFoldersProviding { var feedsAndFolders: [([FeedlyFeed], Folder)] { get } } -/// Single responsibility is accurately reflect Collections from Feedly as Folders. +/// Reflect Collections from Feedly as Folders. final class FeedlyMirrorCollectionsAsFoldersOperation: FeedlyOperation, FeedlyCollectionsAndFoldersProviding, FeedlyFeedsAndFoldersProviding { let account: Account @@ -26,7 +26,7 @@ final class FeedlyMirrorCollectionsAsFoldersOperation: FeedlyOperation, FeedlyCo private(set) var collectionsAndFolders = [(FeedlyCollection, Folder)]() private(set) var feedsAndFolders = [([FeedlyFeed], Folder)]() - + init(account: Account, collectionsProvider: FeedlyCollectionProviding, log: OSLog) { self.collectionsProvider = collectionsProvider self.account = account @@ -34,7 +34,6 @@ final class FeedlyMirrorCollectionsAsFoldersOperation: FeedlyOperation, FeedlyCo } override func run() { - super.run() defer { didFinish() } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyOperation.swift index eeb2fd1f6..5cf77915b 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyOperation.swift @@ -14,60 +14,47 @@ protocol FeedlyOperationDelegate: class { func feedlyOperation(_ operation: FeedlyOperation, didFailWith error: Error) } -/// Abstract class common to all the tasks required to ingest content from Feedly into NetNewsWire. -/// Each task should try to have a single responsibility so they can be easily composed with others. +/// Abstract base class for Feedly sync operations. +/// +/// Normally we don’t do inheritance — but in this case +/// it’s the best option. class FeedlyOperation: MainThreadOperation { weak var delegate: FeedlyOperationDelegate? - - // MainThreadOperationDelegate - var isCanceled = false { - didSet { - if isCanceled { - cancel() - } - } - } - var id: Int? - weak var operationDelegate: MainThreadOperationDelegate? - var completionBlock: FeedlyOperation.MainThreadOperationCompletionBlock? - var name: String? - - var isExecuting = false - var isFinished = false - var downloadProgress: DownloadProgress? { didSet { - guard downloadProgress == nil || !isExecuting else { - fatalError("\(\FeedlyOperation.downloadProgress) was set to late. Set before operation starts executing.") - } oldValue?.completeTask() downloadProgress?.addToNumberOfTasksAndRemaining(1) } } - // Override this. Call super.run() first in the overridden method. - func run() { - isExecuting = true + // MainThreadOperation + var isCanceled = false { + didSet { + if isCanceled { + didCancel() + } + } } + var id: Int? + weak var operationDelegate: MainThreadOperationDelegate? + var name: String? + var completionBlock: MainThreadOperation.MainThreadOperationCompletionBlock? - // Called when isCanceled is set to true. Useful to override. - func cancel() { - didFinish() + func run() { } func didFinish() { - precondition(Thread.isMainThread) - isExecuting = false - isFinished = true - downloadProgress = nil if !isCanceled { operationDelegate?.operationDidComplete(self) } } - - func didFinish(_ error: Error) { + + func didFinish(with error: Error) { delegate?.feedlyOperation(self, didFailWith: error) didFinish() } + + func didCancel() { + } } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyOrganiseParsedItemsByFeedOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyOrganiseParsedItemsByFeedOperation.swift index c6ac74960..a55ea1839 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyOrganiseParsedItemsByFeedOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyOrganiseParsedItemsByFeedOperation.swift @@ -15,8 +15,9 @@ protocol FeedlyParsedItemsByFeedProviding { var parsedItemsKeyedByFeedId: [String: Set] { get } } -/// Single responsibility is to group articles by their feeds. +/// Group articles by their feeds. final class FeedlyOrganiseParsedItemsByFeedOperation: FeedlyOperation, FeedlyParsedItemsByFeedProviding { + private let account: Account private let parsedItemProvider: FeedlyParsedItemProviding private let log: OSLog @@ -42,8 +43,7 @@ final class FeedlyOrganiseParsedItemsByFeedOperation: FeedlyOperation, FeedlyPar defer { didFinish() } - super.run() - + let items = parsedItemProvider.parsedEntries var dict = [String: Set](minimumCapacity: items.count) diff --git a/Frameworks/Account/Feedly/Operations/FeedlyRefreshAccessTokenOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyRefreshAccessTokenOperation.swift index 45db1eb15..d1c68d970 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyRefreshAccessTokenOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyRefreshAccessTokenOperation.swift @@ -11,6 +11,7 @@ import os.log import RSWeb final class FeedlyRefreshAccessTokenOperation: FeedlyOperation { + let service: OAuthAccessTokenRefreshing let oauthClient: OAuthAuthorizationClient let account: Account @@ -24,7 +25,6 @@ final class FeedlyRefreshAccessTokenOperation: FeedlyOperation { } override func run() { - super.run() let refreshToken: Credentials do { @@ -36,7 +36,7 @@ final class FeedlyRefreshAccessTokenOperation: FeedlyOperation { refreshToken = credentials } catch { - didFinish(error) + didFinish(with: error) return } @@ -66,11 +66,11 @@ final class FeedlyRefreshAccessTokenOperation: FeedlyOperation { didFinish() } catch { - didFinish(error) + didFinish(with: error) } case .failure(let error): - didFinish(error) + didFinish(with: error) } } } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyRequestStreamsOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyRequestStreamsOperation.swift index 3c587897d..0cb7731ef 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyRequestStreamsOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyRequestStreamsOperation.swift @@ -13,7 +13,7 @@ protocol FeedlyRequestStreamsOperationDelegate: class { func feedlyRequestStreamsOperation(_ operation: FeedlyRequestStreamsOperation, enqueue collectionStreamOperation: FeedlyGetStreamContentsOperation) } -/// Single responsibility is to create one stream request operation for one Feedly collection. +/// Create one stream request operation for one Feedly collection. /// This is the start of the process of refreshing the entire contents of a Folder. final class FeedlyRequestStreamsOperation: FeedlyOperation { @@ -25,7 +25,7 @@ final class FeedlyRequestStreamsOperation: FeedlyOperation { let log: OSLog let newerThan: Date? let unreadOnly: Bool? - + init(account: Account, collectionsProvider: FeedlyCollectionProviding, newerThan: Date?, unreadOnly: Bool?, service: FeedlyGetStreamContentsService, log: OSLog) { self.account = account self.service = service @@ -36,7 +36,6 @@ final class FeedlyRequestStreamsOperation: FeedlyOperation { } override func run() { - super.run() defer { didFinish() } diff --git a/Frameworks/Account/Feedly/Operations/FeedlySearchOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlySearchOperation.swift index cc492ead1..5e9bfde85 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlySearchOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlySearchOperation.swift @@ -16,15 +16,15 @@ protocol FeedlySearchOperationDelegate: class { func feedlySearchOperation(_ operation: FeedlySearchOperation, didGet response: FeedlyFeedsSearchResponse) } -/// Single responsibility is to find one and only one feed for a given query (usually, a URL). +/// Find one and only one feed for a given query (usually, a URL). /// What happens when a feed is found for the URL is delegated to the `searchDelegate`. class FeedlySearchOperation: FeedlyOperation { + let query: String let locale: Locale let searchService: FeedlySearchService - weak var searchDelegate: FeedlySearchOperationDelegate? - + init(query: String, locale: Locale = .current, service: FeedlySearchService) { self.query = query self.locale = locale @@ -32,8 +32,6 @@ class FeedlySearchOperation: FeedlyOperation { } override func run() { - super.run() - searchService.getFeeds(for: query, count: 1, locale: locale.identifier) { result in switch result { case .success(let response): @@ -42,7 +40,7 @@ class FeedlySearchOperation: FeedlyOperation { self.didFinish() case .failure(let error): - self.didFinish(error) + self.didFinish(with: error) } } } diff --git a/Frameworks/Account/Feedly/Operations/FeedlySendArticleStatusesOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlySendArticleStatusesOperation.swift index 1a1a5850c..e01e6d4c7 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlySendArticleStatusesOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlySendArticleStatusesOperation.swift @@ -11,12 +11,14 @@ import Articles import SyncDatabase import os.log -/// Single responsibility is to take changes to statuses of articles locally and apply them to the corresponding the articles remotely. + +/// Take changes to statuses of articles locally and apply them to the corresponding the articles remotely. final class FeedlySendArticleStatusesOperation: FeedlyOperation { + private let database: SyncDatabase private let log: OSLog private let service: FeedlyMarkArticlesService - + init(database: SyncDatabase, service: FeedlyMarkArticlesService, log: OSLog) { self.database = database self.service = service @@ -24,7 +26,6 @@ final class FeedlySendArticleStatusesOperation: FeedlyOperation { } override func run() { - super.run() os_log(.debug, log: log, "Sending article statuses...") database.selectForProcessing { result in diff --git a/Frameworks/Account/Feedly/Operations/FeedlySyncAllOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlySyncAllOperation.swift index 36b45037d..75282138a 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlySyncAllOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlySyncAllOperation.swift @@ -12,8 +12,9 @@ import SyncDatabase import RSWeb import RSCore -/// Single responsibility is to compose the operations necessary to get the entire set of articles, feeds and folders with the statuses the user expects between now and a certain date in the past. +/// Compose the operations necessary to get the entire set of articles, feeds and folders with the statuses the user expects between now and a certain date in the past. final class FeedlySyncAllOperation: FeedlyOperation { + private let operationQueue = MainThreadOperationQueue() private let log: OSLog let syncUUID: UUID @@ -51,31 +52,31 @@ final class FeedlySyncAllOperation: FeedlyOperation { let getCollections = FeedlyGetCollectionsOperation(service: getCollectionsService, log: log) getCollections.delegate = self getCollections.downloadProgress = downloadProgress - self.operationQueue.make(getCollections, dependOn: sendArticleStatuses) - self.operationQueue.addOperation(getCollections) + getCollections.addDependency(sendArticleStatuses) + self.operationQueue.addOperation(getCollections) // Ensure a folder exists for each Collection, removing Folders without a corresponding Collection. let mirrorCollectionsAsFolders = FeedlyMirrorCollectionsAsFoldersOperation(account: account, collectionsProvider: getCollections, log: log) mirrorCollectionsAsFolders.delegate = self - self.operationQueue.make(mirrorCollectionsAsFolders, dependOn: getCollections) + mirrorCollectionsAsFolders.addDependency(getCollections) self.operationQueue.addOperation(mirrorCollectionsAsFolders) // Ensure feeds are created and grouped by their folders. let createFeedsOperation = FeedlyCreateFeedsForCollectionFoldersOperation(account: account, feedsAndFoldersProvider: mirrorCollectionsAsFolders, log: log) createFeedsOperation.delegate = self - self.operationQueue.make(createFeedsOperation, dependOn: mirrorCollectionsAsFolders) + createFeedsOperation.addDependency(mirrorCollectionsAsFolders) self.operationQueue.addOperation(createFeedsOperation) let getAllArticleIds = FeedlyIngestStreamArticleIdsOperation(account: account, credentials: credentials, service: getStreamIdsService, log: log) getAllArticleIds.delegate = self getAllArticleIds.downloadProgress = downloadProgress - self.operationQueue.make(getAllArticleIds, dependOn: createFeedsOperation) + getAllArticleIds.addDependency(createFeedsOperation) 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, database: database, newerThan: nil, log: log) getUnread.delegate = self - self.operationQueue.make(getUnread, dependOn: getAllArticleIds) + getUnread.addDependency(getAllArticleIds) getUnread.downloadProgress = downloadProgress self.operationQueue.addOperation(getUnread) @@ -84,24 +85,24 @@ final class FeedlySyncAllOperation: FeedlyOperation { let getUpdated = FeedlyGetUpdatedArticleIdsOperation(account: account, credentials: credentials, service: getStreamIdsService, newerThan: lastSuccessfulFetchStartDate, log: log) getUpdated.delegate = self getUpdated.downloadProgress = downloadProgress - self.operationQueue.make(getUpdated, dependOn: createFeedsOperation) + getUpdated.addDependency(createFeedsOperation) self.operationQueue.addOperation(getUpdated) // Get each page of the article ids for starred articles. let getStarred = FeedlyIngestStarredArticleIdsOperation(account: account, credentials: credentials, service: getStarredService, database: database, newerThan: nil, log: log) getStarred.delegate = self getStarred.downloadProgress = downloadProgress - self.operationQueue.make(getStarred, dependOn: createFeedsOperation) + getStarred.addDependency(createFeedsOperation) self.operationQueue.addOperation(getStarred) // Now all the possible article ids we need have a status, fetch the article ids for missing articles. let getMissingIds = FeedlyFetchIdsForMissingArticlesOperation(account: account, log: log) getMissingIds.delegate = self getMissingIds.downloadProgress = downloadProgress - self.operationQueue.make(getMissingIds, dependOn: getAllArticleIds) - self.operationQueue.make(getMissingIds, dependOn: getUnread) - self.operationQueue.make(getMissingIds, dependOn: getStarred) - self.operationQueue.make(getMissingIds, dependOn: getUpdated) + getMissingIds.addDependency(getAllArticleIds) + getMissingIds.addDependency(getUnread) + getMissingIds.addDependency(getStarred) + getMissingIds.addDependency(getUpdated) self.operationQueue.addOperation(getMissingIds) // Download all the missing and updated articles @@ -112,15 +113,15 @@ final class FeedlySyncAllOperation: FeedlyOperation { log: log) downloadMissingArticles.delegate = self downloadMissingArticles.downloadProgress = downloadProgress - self.operationQueue.make(downloadMissingArticles, dependOn: getMissingIds) - self.operationQueue.make(downloadMissingArticles, dependOn: getUpdated) + downloadMissingArticles.addDependency(getMissingIds) + downloadMissingArticles.addDependency(getUpdated) self.operationQueue.addOperation(downloadMissingArticles) // Once this operation's dependencies, their dependencies etc finish, we can finish. let finishOperation = FeedlyCheckpointOperation() finishOperation.checkpointDelegate = self finishOperation.downloadProgress = downloadProgress - self.operationQueue.make(finishOperation, dependOn: downloadMissingArticles) + finishOperation.addDependency(downloadMissingArticles) self.operationQueue.addOperation(finishOperation) } @@ -128,23 +129,16 @@ final class FeedlySyncAllOperation: FeedlyOperation { self.init(account: account, credentials: credentials, lastSuccessfulFetchStartDate: lastSuccessfulFetchStartDate, markArticlesService: caller, getUnreadService: caller, getCollectionsService: caller, getStreamContentsService: caller, getStarredService: caller, getStreamIdsService: caller, getEntriesService: caller, database: database, downloadProgress: downloadProgress, log: log) } - override func cancel() { - os_log(.debug, log: log, "Cancelling sync %{public}@", syncUUID.uuidString) - self.operationQueue.cancelAllOperations() - - super.cancel() - - didFinish() - - // Operation should silently cancel. - syncCompletionHandler = nil - } - override func run() { - super.run() os_log(.debug, log: log, "Starting sync %{public}@", syncUUID.uuidString) operationQueue.resume() } + + override func didCancel() { + os_log(.debug, log: log, "Cancelling sync %{public}@", syncUUID.uuidString) + self.operationQueue.cancelAllOperations() + syncCompletionHandler = nil + } } extension FeedlySyncAllOperation: FeedlyCheckpointOperationDelegate { diff --git a/Frameworks/Account/Feedly/Operations/FeedlySyncStreamContentsOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlySyncStreamContentsOperation.swift index 31ba1ac4f..f41da3524 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlySyncStreamContentsOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlySyncStreamContentsOperation.swift @@ -10,8 +10,10 @@ import Foundation import os.log import RSParser import RSCore +import RSWeb final class FeedlySyncStreamContentsOperation: FeedlyOperation, FeedlyOperationDelegate, FeedlyGetStreamContentsOperationDelegate, FeedlyCheckpointOperationDelegate { + private let account: Account private let resource: FeedlyResourceId private let operationQueue = MainThreadOperationQueue() @@ -43,18 +45,15 @@ final class FeedlySyncStreamContentsOperation: FeedlyOperation, FeedlyOperationD self.init(account: account, resource: all, service: service, isPagingEnabled: true, newerThan: newerThan, log: log) } - override func cancel() { - os_log(.debug, log: log, "Canceling sync stream contents") - operationQueue.cancelAllOperations() - super.cancel() - didFinish() - } - override func run() { - super.run() operationQueue.resume() } - + + override func didCancel() { + os_log(.debug, log: log, "Canceling sync stream contents") + operationQueue.cancelAllOperations() + } + func enqueueOperations(for continuation: String?) { os_log(.debug, log: log, "Requesting page for %@", resource.id) let operations = pageOperations(for: continuation) @@ -70,24 +69,20 @@ final class FeedlySyncStreamContentsOperation: FeedlyOperation, FeedlyOperationD log: log) - let organiseByFeed = FeedlyOrganiseParsedItemsByFeedOperation(account: account, - parsedItemProvider: getPage, - log: log) + let organiseByFeed = FeedlyOrganiseParsedItemsByFeedOperation(account: account, parsedItemProvider: getPage, log: log) - let updateAccount = FeedlyUpdateAccountFeedsWithItemsOperation(account: account, - organisedItemsProvider: organiseByFeed, - log: log) + let updateAccount = FeedlyUpdateAccountFeedsWithItemsOperation(account: account, organisedItemsProvider: organiseByFeed, log: log) getPage.delegate = self getPage.streamDelegate = self - operationQueue.make(organiseByFeed, dependOn: getPage) + organiseByFeed.addDependency(getPage) organiseByFeed.delegate = self - operationQueue.make(updateAccount, dependOn: organiseByFeed) + updateAccount.addDependency(organiseByFeed) updateAccount.delegate = self - operationQueue.make(finishOperation, dependOn: updateAccount) + finishOperation.addDependency(updateAccount) return [getPage, organiseByFeed, updateAccount] } @@ -115,6 +110,6 @@ final class FeedlySyncStreamContentsOperation: FeedlyOperation, FeedlyOperationD func feedlyOperation(_ operation: FeedlyOperation, didFailWith error: Error) { operationQueue.cancelAllOperations() - didFinish(error) + didFinish(with: error) } } diff --git a/Frameworks/Account/Feedly/Operations/FeedlyUpdateAccountFeedsWithItemsOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyUpdateAccountFeedsWithItemsOperation.swift index 76f423bb8..b15d17cf9 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyUpdateAccountFeedsWithItemsOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyUpdateAccountFeedsWithItemsOperation.swift @@ -10,12 +10,13 @@ import Foundation import RSParser import os.log -/// Single responsibility is to combine the articles with their feeds for a specific account. +/// Combine the articles with their feeds for a specific account. final class FeedlyUpdateAccountFeedsWithItemsOperation: FeedlyOperation { + private let account: Account private let organisedItemsProvider: FeedlyParsedItemsByFeedProviding private let log: OSLog - + init(account: Account, organisedItemsProvider: FeedlyParsedItemsByFeedProviding, log: OSLog) { self.account = account self.organisedItemsProvider = organisedItemsProvider @@ -23,12 +24,11 @@ final class FeedlyUpdateAccountFeedsWithItemsOperation: FeedlyOperation { } override func run() { - super.run() let webFeedIDsAndItems = organisedItemsProvider.parsedItemsKeyedByFeedId account.update(webFeedIDsAndItems: webFeedIDsAndItems, defaultRead: true) { databaseError in if let error = databaseError { - self.didFinish(error) + self.didFinish(with: error) return }