From 3ed5a43de3af604abf7e599972eb47a6ab0dbf6b Mon Sep 17 00:00:00 2001 From: Kiel Gillard Date: Fri, 15 Nov 2019 07:59:44 +1100 Subject: [PATCH] Improves the behaviour and fixes some issues with cancelling of Feedly operations. --- .../Feedly/Operations/FeedlyOperation.swift | 14 ++++++++------ .../Feedly/Operations/FeedlySyncAllOperation.swift | 5 ++--- .../FeedlySyncStarredArticlesOperation.swift | 9 ++++++++- .../FeedlySyncStreamContentsOperation.swift | 9 ++++++++- .../FeedlySyncUnreadStatusesOperation.swift | 9 ++++++++- 5 files changed, 34 insertions(+), 12 deletions(-) diff --git a/Frameworks/Account/Feedly/Operations/FeedlyOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyOperation.swift index 8c231bc53..5c6b16afc 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlyOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlyOperation.swift @@ -21,8 +21,8 @@ class FeedlyOperation: Operation { func didFinish() { assert(Thread.isMainThread) assert(!isFinished, "Finished operation is attempting to finish again.") - self.isExecutingOperation = false - self.isFinishedOperation = true + isExecutingOperation = false + isFinishedOperation = true } func didFinish(_ error: Error) { @@ -33,16 +33,18 @@ class FeedlyOperation: Operation { } override func start() { + guard !isCancelled else { + isExecutingOperation = false + isFinishedOperation = true + return + } + isExecutingOperation = true DispatchQueue.main.async { self.main() } } - override func cancel() { - super.cancel() - } - override var isExecuting: Bool { return isExecutingOperation } diff --git a/Frameworks/Account/Feedly/Operations/FeedlySyncAllOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlySyncAllOperation.swift index 43faee339..b93031677 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlySyncAllOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlySyncAllOperation.swift @@ -94,10 +94,9 @@ final class FeedlySyncAllOperation: FeedlyOperation { os_log(.debug, log: log, "Cancelling sync %{public}@", syncUUID.uuidString) self.operationQueue.cancelAllOperations() - syncCompletionHandler?(.failure(URLError(.cancelled))) - syncCompletionHandler = nil + super.cancel() - self.didFinish() + didFinish() } override func main() { diff --git a/Frameworks/Account/Feedly/Operations/FeedlySyncStarredArticlesOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlySyncStarredArticlesOperation.swift index c4955362b..ea608ccbb 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlySyncStarredArticlesOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlySyncStarredArticlesOperation.swift @@ -96,13 +96,15 @@ final class FeedlySyncStarredArticlesOperation: FeedlyOperation, FeedlyOperation } override func cancel() { + os_log(.debug, log: log, "Canceling sync starred articles") operationQueue.cancelAllOperations() super.cancel() + didFinish() } override func main() { guard !isCancelled else { - didFinish() + // override of cancel calls didFinish(). return } @@ -110,6 +112,11 @@ final class FeedlySyncStarredArticlesOperation: FeedlyOperation, FeedlyOperation } func feedlyGetStreamContentsOperation(_ operation: FeedlyGetStreamContentsOperation, didGetContentsOf stream: FeedlyStream) { + guard !isCancelled else { + os_log(.debug, log: log, "Cancelled starred stream contents for %@", stream.id) + return + } + entryProvider.addEntries(from: operation) os_log(.debug, log: log, "Collecting %i items from %@", stream.items.count, stream.id) diff --git a/Frameworks/Account/Feedly/Operations/FeedlySyncStreamContentsOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlySyncStreamContentsOperation.swift index 2d416b697..667857073 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlySyncStreamContentsOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlySyncStreamContentsOperation.swift @@ -42,13 +42,15 @@ final class FeedlySyncStreamContentsOperation: FeedlyOperation, FeedlyOperationD } override func cancel() { + os_log(.debug, log: log, "Canceling sync stream contents") operationQueue.cancelAllOperations() super.cancel() + didFinish() } override func main() { guard !isCancelled else { - didFinish() + // override of cancel calls didFinish(). return } @@ -92,6 +94,11 @@ final class FeedlySyncStreamContentsOperation: FeedlyOperation, FeedlyOperationD } func feedlyGetStreamContentsOperation(_ operation: FeedlyGetStreamContentsOperation, didGetContentsOf stream: FeedlyStream) { + guard !isCancelled else { + os_log(.debug, log: log, "Cancelled requesting page for %@", resource.id) + return + } + os_log(.debug, log: log, "Ingesting %i items from %@", stream.items.count, stream.id) guard let continuation = stream.continuation else { diff --git a/Frameworks/Account/Feedly/Operations/FeedlySyncUnreadStatusesOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlySyncUnreadStatusesOperation.swift index 15280aa44..6cae9d468 100644 --- a/Frameworks/Account/Feedly/Operations/FeedlySyncUnreadStatusesOperation.swift +++ b/Frameworks/Account/Feedly/Operations/FeedlySyncUnreadStatusesOperation.swift @@ -77,13 +77,15 @@ final class FeedlySyncUnreadStatusesOperation: FeedlyOperation, FeedlyOperationD } override func cancel() { + os_log(.debug, log: log, "Canceling sync unread statuses") operationQueue.cancelAllOperations() super.cancel() + didFinish() } override func main() { guard !isCancelled else { - didFinish() + // override of cancel calls didFinish(). return } @@ -91,6 +93,11 @@ final class FeedlySyncUnreadStatusesOperation: FeedlyOperation, FeedlyOperationD } func feedlyGetStreamIdsOperation(_ operation: FeedlyGetStreamIdsOperation, didGet streamIds: FeedlyStreamIds) { + guard !isCancelled else { + os_log(.debug, log: log, "Cancelled unread stream ids.") + return + } + os_log(.debug, log: log, "Collecting %i unread article ids from %@", streamIds.ids.count, resource.id) unreadEntryIdsProvider.addEntryIds(from: operation)