From f159371967020ac657d2c113262b22b6b39980e9 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Mon, 2 Dec 2019 14:14:35 -0600 Subject: [PATCH] Change to make sure all queue's get cleared before suspending the database. Issue #1389 --- Frameworks/Account/Account.swift | 1 - Frameworks/Account/AccountDelegate.swift | 1 - .../Account/Feedbin/FeedbinAPICaller.swift | 196 +++++++++++++++++- .../Feedbin/FeedbinAccountDelegate.swift | 6 +- .../Feedly/FeedlyAccountDelegate.swift | 5 +- .../ReaderAPI/ReaderAPIAccountDelegate.swift | 5 +- Shared/Timeline/FetchRequestOperation.swift | 6 - iOS/AppDelegate.swift | 17 +- iOS/SceneCoordinator.swift | 5 + iOS/SceneDelegate.swift | 4 + submodules/RSWeb | 2 +- 11 files changed, 213 insertions(+), 35 deletions(-) diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index bc7808a75..956c2bb96 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -406,7 +406,6 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, } public func suspend() { - delegate.cancelAll(for: self) delegate.suspend() database.suspend() save() diff --git a/Frameworks/Account/AccountDelegate.swift b/Frameworks/Account/AccountDelegate.swift index 9ddecdbb7..647921795 100644 --- a/Frameworks/Account/AccountDelegate.swift +++ b/Frameworks/Account/AccountDelegate.swift @@ -22,7 +22,6 @@ protocol AccountDelegate { var refreshProgress: DownloadProgress { get } - func cancelAll(for account: Account) func refreshAll(for account: Account, completion: @escaping (Result) -> Void) func sendArticleStatus(for account: Account, completion: @escaping ((Result) -> Void)) func refreshArticleStatus(for account: Account, completion: @escaping ((Result) -> Void)) diff --git a/Frameworks/Account/Feedbin/FeedbinAPICaller.swift b/Frameworks/Account/Feedbin/FeedbinAPICaller.swift index 912187cde..b6b2b824a 100644 --- a/Frameworks/Account/Feedbin/FeedbinAPICaller.swift +++ b/Frameworks/Account/Feedbin/FeedbinAPICaller.swift @@ -32,6 +32,7 @@ final class FeedbinAPICaller: NSObject { private let feedbinBaseURL = URL(string: "https://api.feedbin.com/v2/")! private var transport: Transport! + private var suspended = false var credentials: Credentials? weak var accountMetadata: AccountMetadata? @@ -41,8 +42,14 @@ final class FeedbinAPICaller: NSObject { self.transport = transport } - func cancelAll() { + /// Cancels all pending requests rejects any that come in later + func suspend() { transport.cancelAll() + suspended = true + } + + func resume() { + suspended = false } func validateCredentials(completion: @escaping (Result) -> Void) { @@ -51,6 +58,12 @@ final class FeedbinAPICaller: NSObject { let request = URLRequest(url: callURL, credentials: credentials) transport.send(request: request) { result in + + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + switch result { case .success: completion(.success(self.credentials)) @@ -78,6 +91,11 @@ final class FeedbinAPICaller: NSObject { transport.send(request: request, method: HTTPMethod.post, payload: opmlData) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + switch result { case .success(let (_, data)): @@ -108,6 +126,11 @@ final class FeedbinAPICaller: NSObject { transport.send(request: request, resultType: FeedbinImportResult.self) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + switch result { case .success(let (_, importResult)): completion(.success(importResult)) @@ -127,6 +150,11 @@ final class FeedbinAPICaller: NSObject { transport.send(request: request, resultType: [FeedbinTag].self) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + switch result { case .success(let (response, tags)): self.storeConditionalGet(key: ConditionalGetKeys.tags, headers: response.allHeaderFields) @@ -143,7 +171,20 @@ final class FeedbinAPICaller: NSObject { let callURL = feedbinBaseURL.appendingPathComponent("tags.json") let request = URLRequest(url: callURL, credentials: credentials) let payload = FeedbinRenameTag(oldName: oldName, newName: newName) - transport.send(request: request, method: HTTPMethod.post, payload: payload, completion: completion) + + transport.send(request: request, method: HTTPMethod.post, payload: payload) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + + switch result { + case .success: + completion(.success(())) + case .failure(let error): + completion(.failure(error)) + } + } } func retrieveSubscriptions(completion: @escaping (Result<[FeedbinSubscription]?, Error>) -> Void) { @@ -156,6 +197,11 @@ final class FeedbinAPICaller: NSObject { transport.send(request: request, resultType: [FeedbinSubscription].self) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + switch result { case .success(let (response, subscriptions)): self.storeConditionalGet(key: ConditionalGetKeys.subscriptions, headers: response.allHeaderFields) @@ -186,6 +232,11 @@ final class FeedbinAPICaller: NSObject { transport.send(request: request, method: HTTPMethod.post, payload: payload) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + switch result { case .success(let (response, data)): @@ -246,13 +297,38 @@ final class FeedbinAPICaller: NSObject { let callURL = feedbinBaseURL.appendingPathComponent("subscriptions/\(subscriptionID)/update.json") let request = URLRequest(url: callURL, credentials: credentials) let payload = FeedbinUpdateSubscription(title: newName) - transport.send(request: request, method: HTTPMethod.post, payload: payload, completion: completion) + + transport.send(request: request, method: HTTPMethod.post, payload: payload) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + + switch result { + case .success: + completion(.success(())) + case .failure(let error): + completion(.failure(error)) + } + } } func deleteSubscription(subscriptionID: String, completion: @escaping (Result) -> Void) { let callURL = feedbinBaseURL.appendingPathComponent("subscriptions/\(subscriptionID).json") let request = URLRequest(url: callURL, credentials: credentials) - transport.send(request: request, method: HTTPMethod.delete, completion: completion) + transport.send(request: request, method: HTTPMethod.delete) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + + switch result { + case .success: + completion(.success(())) + case .failure(let error): + completion(.failure(error)) + } + } } func retrieveTaggings(completion: @escaping (Result<[FeedbinTagging]?, Error>) -> Void) { @@ -262,7 +338,11 @@ final class FeedbinAPICaller: NSObject { let request = URLRequest(url: callURL, credentials: credentials, conditionalGet: conditionalGet) transport.send(request: request, resultType: [FeedbinTagging].self) { result in - + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + switch result { case .success(let (response, taggings)): self.storeConditionalGet(key: ConditionalGetKeys.taggings, headers: response.allHeaderFields) @@ -291,6 +371,11 @@ final class FeedbinAPICaller: NSObject { transport.send(request: request, method: HTTPMethod.post, payload:payload) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + switch result { case .success(let (response, _)): if let taggingLocation = response.valueForHTTPHeaderField(HTTPResponseHeader.location), @@ -313,7 +398,19 @@ final class FeedbinAPICaller: NSObject { let callURL = feedbinBaseURL.appendingPathComponent("taggings/\(taggingID).json") var request = URLRequest(url: callURL, credentials: credentials) request.addValue("application/json; charset=utf-8", forHTTPHeaderField: HTTPRequestHeader.contentType) - transport.send(request: request, method: HTTPMethod.delete, completion: completion) + transport.send(request: request, method: HTTPMethod.delete) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + + switch result { + case .success: + completion(.success(())) + case .failure(let error): + completion(.failure(error)) + } + } } func retrieveEntries(articleIDs: [String], completion: @escaping (Result<([FeedbinEntry]?), Error>) -> Void) { @@ -336,6 +433,11 @@ final class FeedbinAPICaller: NSObject { transport.send(request: request, resultType: [FeedbinEntry].self) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + switch result { case .success(let (_, entries)): completion(.success((entries))) @@ -363,6 +465,11 @@ final class FeedbinAPICaller: NSObject { transport.send(request: request, resultType: [FeedbinEntry].self) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + switch result { case .success(let (response, entries)): @@ -399,6 +506,11 @@ final class FeedbinAPICaller: NSObject { transport.send(request: request, resultType: [FeedbinEntry].self) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + switch result { case .success(let (response, entries)): @@ -427,6 +539,11 @@ final class FeedbinAPICaller: NSObject { transport.send(request: request, resultType: [FeedbinEntry].self) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + switch result { case .success(let (response, entries)): @@ -449,6 +566,11 @@ final class FeedbinAPICaller: NSObject { transport.send(request: request, resultType: [Int].self) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + switch result { case .success(let (response, unreadEntries)): self.storeConditionalGet(key: ConditionalGetKeys.unreadEntries, headers: response.allHeaderFields) @@ -465,14 +587,38 @@ final class FeedbinAPICaller: NSObject { let callURL = feedbinBaseURL.appendingPathComponent("unread_entries.json") let request = URLRequest(url: callURL, credentials: credentials) let payload = FeedbinUnreadEntry(unreadEntries: entries) - transport.send(request: request, method: HTTPMethod.post, payload: payload, completion: completion) + transport.send(request: request, method: HTTPMethod.post, payload: payload) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + + switch result { + case .success: + completion(.success(())) + case .failure(let error): + completion(.failure(error)) + } + } } func deleteUnreadEntries(entries: [Int], completion: @escaping (Result) -> Void) { let callURL = feedbinBaseURL.appendingPathComponent("unread_entries.json") let request = URLRequest(url: callURL, credentials: credentials) let payload = FeedbinUnreadEntry(unreadEntries: entries) - transport.send(request: request, method: HTTPMethod.delete, payload: payload, completion: completion) + transport.send(request: request, method: HTTPMethod.delete, payload: payload) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + + switch result { + case .success: + completion(.success(())) + case .failure(let error): + completion(.failure(error)) + } + } } func retrieveStarredEntries(completion: @escaping (Result<[Int]?, Error>) -> Void) { @@ -482,7 +628,11 @@ final class FeedbinAPICaller: NSObject { let request = URLRequest(url: callURL, credentials: credentials, conditionalGet: conditionalGet) transport.send(request: request, resultType: [Int].self) { result in - + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + switch result { case .success(let (response, starredEntries)): self.storeConditionalGet(key: ConditionalGetKeys.starredEntries, headers: response.allHeaderFields) @@ -499,14 +649,38 @@ final class FeedbinAPICaller: NSObject { let callURL = feedbinBaseURL.appendingPathComponent("starred_entries.json") let request = URLRequest(url: callURL, credentials: credentials) let payload = FeedbinStarredEntry(starredEntries: entries) - transport.send(request: request, method: HTTPMethod.post, payload: payload, completion: completion) + transport.send(request: request, method: HTTPMethod.post, payload: payload) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + + switch result { + case .success: + completion(.success(())) + case .failure(let error): + completion(.failure(error)) + } + } } func deleteStarredEntries(entries: [Int], completion: @escaping (Result) -> Void) { let callURL = feedbinBaseURL.appendingPathComponent("starred_entries.json") let request = URLRequest(url: callURL, credentials: credentials) let payload = FeedbinStarredEntry(starredEntries: entries) - transport.send(request: request, method: HTTPMethod.delete, payload: payload, completion: completion) + transport.send(request: request, method: HTTPMethod.delete, payload: payload) { result in + if self.suspended { + completion(.failure(TransportError.suspended)) + return + } + + switch result { + case .success: + completion(.success(())) + case .failure(let error): + completion(.failure(error)) + } + } } } diff --git a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift index cc1dfa709..c493a3b14 100644 --- a/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift +++ b/Frameworks/Account/Feedbin/FeedbinAccountDelegate.swift @@ -73,10 +73,6 @@ final class FeedbinAccountDelegate: AccountDelegate { var refreshProgress = DownloadProgress(numberOfTasks: 0) - func cancelAll(for account: Account) { - caller.cancelAll() - } - func refreshAll(for account: Account, completion: @escaping (Result) -> Void) { refreshProgress.addToNumberOfTasksAndRemaining(5) @@ -560,11 +556,13 @@ final class FeedbinAccountDelegate: AccountDelegate { /// Suspend the sync database so that it can close its SQLite file. func suspend() { + caller.suspend() database.suspend() } /// Resume the sync database — let it reopen its SQLite file. func resume() { + caller.resume() database.resume() } } diff --git a/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift b/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift index 48db9fa41..1277630d7 100644 --- a/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift +++ b/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift @@ -97,10 +97,6 @@ final class FeedlyAccountDelegate: AccountDelegate { // MARK: Account API - func cancelAll(for account: Account) { - operationQueue.cancelAllOperations() - } - func refreshAll(for account: Account, completion: @escaping (Result) -> Void) { assert(Thread.isMainThread) @@ -515,6 +511,7 @@ final class FeedlyAccountDelegate: AccountDelegate { /// Suspend the sync database so that it can close its SQLite file. func suspend() { + operationQueue.cancelAllOperations() database.suspend() } diff --git a/Frameworks/Account/ReaderAPI/ReaderAPIAccountDelegate.swift b/Frameworks/Account/ReaderAPI/ReaderAPIAccountDelegate.swift index 493a29e2c..488332edd 100644 --- a/Frameworks/Account/ReaderAPI/ReaderAPIAccountDelegate.swift +++ b/Frameworks/Account/ReaderAPI/ReaderAPIAccountDelegate.swift @@ -79,10 +79,6 @@ final class ReaderAPIAccountDelegate: AccountDelegate { var refreshProgress = DownloadProgress(numberOfTasks: 0) - func cancelAll(for account: Account) { - caller.cancelAll() - } - func refreshAll(for account: Account, completion: @escaping (Result) -> Void) { refreshProgress.addToNumberOfTasksAndRemaining(6) @@ -441,6 +437,7 @@ final class ReaderAPIAccountDelegate: AccountDelegate { /// Suspend the sync database so that it can close its SQLite file. func suspend() { + caller.cancelAll() database.suspend() } diff --git a/Shared/Timeline/FetchRequestOperation.swift b/Shared/Timeline/FetchRequestOperation.swift index ee1334cd6..08f9cddc0 100644 --- a/Shared/Timeline/FetchRequestOperation.swift +++ b/Shared/Timeline/FetchRequestOperation.swift @@ -46,12 +46,6 @@ final class FetchRequestOperation { } } - // The account manager may have been suspended while we were queued up - if AccountManager.shared.isSuspended { - callCompletionIfNeeded() - return - } - if isCanceled { callCompletionIfNeeded() return diff --git a/iOS/AppDelegate.swift b/iOS/AppDelegate.swift index 604e079b2..11a941a3f 100644 --- a/iOS/AppDelegate.swift +++ b/iOS/AppDelegate.swift @@ -229,6 +229,7 @@ private extension AppDelegate { } // MARK: Go To Background + private extension AppDelegate { func waitForSyncTasksToFinish() { @@ -269,7 +270,7 @@ private extension AppDelegate { func completeProcessing(_ suspend: Bool) { if suspend { - AccountManager.shared.suspendAll() + suspendApplication() } UIApplication.shared.endBackgroundTask(self.waitBackgroundUpdateTask) self.waitBackgroundUpdateTask = UIBackgroundTaskIdentifier.invalid @@ -299,6 +300,16 @@ private extension AppDelegate { } } + func suspendApplication() { + CoalescingQueue.standard.performCallsImmediately() + for scene in UIApplication.shared.connectedScenes { + if let sceneDelegate = scene.delegate as? SceneDelegate { + sceneDelegate.suspend() + } + } + AccountManager.shared.suspendAll() + } + } // MARK: Background Tasks @@ -342,9 +353,9 @@ private extension AppDelegate { if AccountManager.shared.isSuspended { AccountManager.shared.resumeAll() } - AccountManager.shared.refreshAll(errorHandler: ErrorHandler.log) { + AccountManager.shared.refreshAll(errorHandler: ErrorHandler.log) { [unowned self] in AccountManager.shared.saveAll() - AccountManager.shared.suspendAll() + self.suspendApplication() os_log("Account refresh operation completed.", log: self.log, type: .info) task?.setTaskCompleted(success: true) } diff --git a/iOS/SceneCoordinator.swift b/iOS/SceneCoordinator.swift index 7c58a4d84..6eee82719 100644 --- a/iOS/SceneCoordinator.swift +++ b/iOS/SceneCoordinator.swift @@ -535,6 +535,11 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { // MARK: API + func suspend() { + fetchAndMergeArticlesQueue.performCallsImmediately() + fetchRequestQueue.cancelAllRequests() + } + func shadowNodesFor(section: Int) -> [Node] { return shadowTable[section] } diff --git a/iOS/SceneDelegate.swift b/iOS/SceneDelegate.swift index d88e38d61..fa289c74f 100644 --- a/iOS/SceneDelegate.swift +++ b/iOS/SceneDelegate.swift @@ -73,6 +73,10 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate { coordinator.handle(response) } + func suspend() { + coordinator.suspend() + } + } private extension SceneDelegate { diff --git a/submodules/RSWeb b/submodules/RSWeb index b2ef3e111..0631696e7 160000 --- a/submodules/RSWeb +++ b/submodules/RSWeb @@ -1 +1 @@ -Subproject commit b2ef3e111339ba9dd9070ed0ded8e053135559cb +Subproject commit 0631696e7383368664a1fe47c58ebcd0ee2d49f8