Rework download progress so that the delegate always manages it to make for smoother progress bar progressions.

This commit is contained in:
Maurice Parker 2020-04-02 12:00:10 -05:00
parent 40ea5243c6
commit 2924c0e6cc
7 changed files with 114 additions and 76 deletions

View File

@ -43,9 +43,7 @@ final class CloudKitAccountDelegate: AccountDelegate {
var credentials: Credentials?
var accountMetadata: AccountMetadata?
var refreshProgress: DownloadProgress {
return refresher.progress
}
var refreshProgress = DownloadProgress(numberOfTasks: 0)
init(dataFolder: String) {
accountZone = CloudKitAccountZone(container: container)
@ -53,8 +51,6 @@ final class CloudKitAccountDelegate: AccountDelegate {
let databaseFilePath = (dataFolder as NSString).appendingPathComponent("Sync.sqlite3")
database = SyncDatabase(databaseFilePath: databaseFilePath)
accountZone.refreshProgress = refreshProgress
}
func receiveRemoteNotification(for account: Account, userInfo: [AnyHashable : Any], completion: @escaping () -> Void) {
@ -76,39 +72,7 @@ final class CloudKitAccountDelegate: AccountDelegate {
}
func refreshAll(for account: Account, completion: @escaping (Result<Void, Error>) -> Void) {
BatchUpdate.shared.start()
accountZone.fetchChangesInZone() { result in
BatchUpdate.shared.end()
switch result {
case .success:
self.sendArticleStatus(for: account) { result in
switch result {
case .success:
self.refreshArticleStatus(for: account) { result in
switch result {
case .success:
self.refresher.refreshFeeds(account.flattenedWebFeeds()) {
account.metadata.lastArticleFetchEndTime = Date()
completion(.success(()))
}
case .failure(let error):
completion(.failure(error))
}
}
case .failure(let error):
completion(.failure(error))
}
}
case .failure(let error):
BatchUpdate.shared.end()
completion(.failure(error))
}
}
refreshAll(for: account, downloadFeeds: true, completion: completion)
}
func sendArticleStatus(for account: Account, completion: @escaping ((Result<Void, Error>) -> Void)) {
@ -204,9 +168,10 @@ final class CloudKitAccountDelegate: AccountDelegate {
return
}
refreshProgress.addToNumberOfTasksAndRemaining(1)
refreshProgress.addToNumberOfTasksAndRemaining(3)
FeedFinder.find(url: url) { result in
self.refreshProgress.completeTask()
switch result {
case .success(let feedSpecifiers):
guard let bestFeedSpecifier = FeedSpecifier.bestFeed(in: feedSpecifiers), let url = URL(string: bestFeedSpecifier.urlString) else {
@ -222,6 +187,8 @@ final class CloudKitAccountDelegate: AccountDelegate {
}
self.accountZone.createWebFeed(url: bestFeedSpecifier.urlString, editedName: name, container: container) { result in
self.refreshProgress.completeTask()
switch result {
case .success(let externalID):
@ -242,13 +209,12 @@ final class CloudKitAccountDelegate: AccountDelegate {
}
case .failure(let error):
self.refreshProgress.completeTask()
completion(.failure(error))
}
}
case .failure:
self.refreshProgress.completeTask()
self.refreshProgress.clear()
completion(.failure(AccountError.createErrorNotFound))
}
@ -258,7 +224,9 @@ final class CloudKitAccountDelegate: AccountDelegate {
func renameWebFeed(for account: Account, with feed: WebFeed, to name: String, completion: @escaping (Result<Void, Error>) -> Void) {
let editedName = name.isEmpty ? nil : name
refreshProgress.addToNumberOfTasksAndRemaining(1)
accountZone.renameWebFeed(feed, editedName: editedName) { result in
self.refreshProgress.completeTask()
switch result {
case .success:
feed.editedName = name
@ -270,7 +238,9 @@ final class CloudKitAccountDelegate: AccountDelegate {
}
func removeWebFeed(for account: Account, with feed: WebFeed, from container: Container, completion: @escaping (Result<Void, Error>) -> Void) {
refreshProgress.addToNumberOfTasksAndRemaining(1)
accountZone.removeWebFeed(feed, from: container) { result in
self.refreshProgress.completeTask()
switch result {
case .success:
container.removeWebFeed(feed)
@ -282,7 +252,9 @@ final class CloudKitAccountDelegate: AccountDelegate {
}
func moveWebFeed(for account: Account, with feed: WebFeed, from fromContainer: Container, to toContainer: Container, completion: @escaping (Result<Void, Error>) -> Void) {
refreshProgress.addToNumberOfTasksAndRemaining(1)
accountZone.moveWebFeed(feed, from: fromContainer, to: toContainer) { result in
self.refreshProgress.completeTask()
switch result {
case .success:
fromContainer.removeWebFeed(feed)
@ -295,7 +267,9 @@ final class CloudKitAccountDelegate: AccountDelegate {
}
func addWebFeed(for account: Account, with feed: WebFeed, to container: Container, completion: @escaping (Result<Void, Error>) -> Void) {
refreshProgress.addToNumberOfTasksAndRemaining(1)
accountZone.addWebFeed(feed, to: container) { result in
self.refreshProgress.completeTask()
switch result {
case .success:
container.addWebFeed(feed)
@ -307,7 +281,9 @@ final class CloudKitAccountDelegate: AccountDelegate {
}
func restoreWebFeed(for account: Account, feed: WebFeed, container: Container, completion: @escaping (Result<Void, Error>) -> Void) {
refreshProgress.addToNumberOfTasksAndRemaining(1)
accountZone.createWebFeed(url: feed.url, editedName: feed.editedName, container: container) { result in
self.refreshProgress.completeTask()
switch result {
case .success(let externalID):
feed.externalID = externalID
@ -320,7 +296,9 @@ final class CloudKitAccountDelegate: AccountDelegate {
}
func createFolder(for account: Account, name: String, completion: @escaping (Result<Folder, Error>) -> Void) {
refreshProgress.addToNumberOfTasksAndRemaining(1)
accountZone.createFolder(name: name) { result in
self.refreshProgress.completeTask()
switch result {
case .success(let externalID):
if let folder = account.ensureFolder(with: name) {
@ -336,7 +314,9 @@ final class CloudKitAccountDelegate: AccountDelegate {
}
func renameFolder(for account: Account, with folder: Folder, to name: String, completion: @escaping (Result<Void, Error>) -> Void) {
refreshProgress.addToNumberOfTasksAndRemaining(1)
accountZone.renameFolder(folder, to: name) { result in
self.refreshProgress.completeTask()
switch result {
case .success:
folder.name = name
@ -348,7 +328,9 @@ final class CloudKitAccountDelegate: AccountDelegate {
}
func removeFolder(for account: Account, with folder: Folder, completion: @escaping (Result<Void, Error>) -> Void) {
refreshProgress.addToNumberOfTasksAndRemaining(1)
accountZone.removeFolder(folder) { result in
self.refreshProgress.completeTask()
switch result {
case .success:
account.removeFolder(folder)
@ -365,19 +347,24 @@ final class CloudKitAccountDelegate: AccountDelegate {
return
}
let feedsToRestore = folder.topLevelWebFeeds
refreshProgress.addToNumberOfTasksAndRemaining(1 + feedsToRestore.count)
accountZone.createFolder(name: name) { result in
self.refreshProgress.completeTask()
switch result {
case .success(let externalID):
folder.externalID = externalID
account.addFolder(folder)
let group = DispatchGroup()
for feed in folder.topLevelWebFeeds {
for feed in feedsToRestore {
folder.topLevelWebFeeds.remove(feed)
group.enter()
self.restoreWebFeed(for: account, feed: feed, container: folder) { result in
self.refreshProgress.completeTask()
group.leave()
switch result {
case .success:
@ -424,7 +411,7 @@ final class CloudKitAccountDelegate: AccountDelegate {
switch result {
case .success(let externalID):
account.externalID = externalID
self.refreshAll(for: account) { result in
self.refreshAll(for: account, downloadFeeds: false) { result in
if case .failure(let error) = result {
os_log(.error, log: self.log, "Error while doing intial refresh: %@", error.localizedDescription)
}
@ -466,3 +453,64 @@ final class CloudKitAccountDelegate: AccountDelegate {
database.resume()
}
}
private extension CloudKitAccountDelegate {
func refreshAll(for account: Account, downloadFeeds: Bool, completion: @escaping (Result<Void, Error>) -> Void) {
let intialWebFeedsCount = downloadFeeds ? account.flattenedWebFeeds().count : 0
refreshProgress.addToNumberOfTasksAndRemaining(3 + intialWebFeedsCount)
BatchUpdate.shared.start()
accountZone.fetchChangesInZone() { result in
BatchUpdate.shared.end()
switch result {
case .success:
let webFeeds = account.flattenedWebFeeds()
if downloadFeeds {
self.refreshProgress.addToNumberOfTasksAndRemaining(webFeeds.count - intialWebFeedsCount)
}
self.refreshProgress.completeTask()
self.sendArticleStatus(for: account) { result in
switch result {
case .success:
self.refreshProgress.completeTask()
self.refreshArticleStatus(for: account) { result in
switch result {
case .success:
self.refreshProgress.completeTask()
guard downloadFeeds else {
completion(.success(()))
return
}
self.refresher.refreshFeeds(webFeeds, feedCompletionBlock: { _ in self.refreshProgress.completeTask() }) {
account.metadata.lastArticleFetchEndTime = Date()
completion(.success(()))
}
case .failure(let error):
completion(.failure(error))
}
}
case .failure(let error):
completion(.failure(error))
}
}
case .failure(let error):
self.refreshProgress.clear()
BatchUpdate.shared.end()
completion(.failure(error))
}
}
}
}

View File

@ -22,7 +22,6 @@ final class CloudKitAccountZone: CloudKitZone {
weak var container: CKContainer?
weak var database: CKDatabase?
weak var refreshProgress: DownloadProgress?
var delegate: CloudKitZoneDelegate?
struct CloudKitWebFeed {

View File

@ -162,7 +162,6 @@ private extension CloudKitAcountZoneDelegate {
return webFeed
}
func addUnclaimedWebFeed(url: URL, editedName: String?, webFeedExternalID: String, containerExternalID: String) {
if var unclaimedWebFeeds = self.unclaimedWebFeeds[containerExternalID] {
unclaimedWebFeeds.append(UnclaimedWebFeed(url: url, editedName: editedName, webFeedExternalID: webFeedExternalID))

View File

@ -22,7 +22,6 @@ final class CloudKitArticlesZone: CloudKitZone {
weak var container: CKContainer?
weak var database: CKDatabase?
weak var refreshProgress: DownloadProgress? = nil
var delegate: CloudKitZoneDelegate? = nil
struct CloudKitArticleStatus {

View File

@ -32,7 +32,6 @@ protocol CloudKitZone: class {
var container: CKContainer? { get }
var database: CKDatabase? { get }
var refreshProgress: DownloadProgress? { get set }
var delegate: CloudKitZoneDelegate? { get set }
}
@ -105,13 +104,10 @@ extension CloudKitZone {
return
}
refreshProgress?.addToNumberOfTasksAndRemaining(1)
database.perform(query, inZoneWith: Self.zoneID) { [weak self] records, error in
switch CloudKitZoneResult.resolve(error) {
case .success:
DispatchQueue.main.async {
self?.refreshProgress?.completeTask()
if let records = records {
completion(.success(records))
} else {
@ -124,7 +120,6 @@ extension CloudKitZone {
}
default:
DispatchQueue.main.async {
self?.refreshProgress?.completeTask()
completion(.failure(error!))
}
}
@ -139,12 +134,10 @@ extension CloudKitZone {
let recordID = CKRecord.ID(recordName: externalID, zoneID: Self.zoneID)
refreshProgress?.addToNumberOfTasksAndRemaining(1)
database?.fetch(withRecordID: recordID) { [weak self] record, error in
switch CloudKitZoneResult.resolve(error) {
case .success:
DispatchQueue.main.async {
self?.refreshProgress?.completeTask()
if let record = record {
completion(.success(record))
} else {
@ -157,7 +150,6 @@ extension CloudKitZone {
}
default:
DispatchQueue.main.async {
self?.refreshProgress?.completeTask()
completion(.failure(error!))
}
}
@ -201,7 +193,6 @@ extension CloudKitZone {
switch CloudKitZoneResult.resolve(error) {
case .success:
DispatchQueue.main.async {
self.refreshProgress?.completeTask()
completion(.success(()))
}
case .zoneNotFound:
@ -211,14 +202,12 @@ extension CloudKitZone {
self.modify(recordsToSave: recordsToSave, recordIDsToDelete: recordIDsToDelete, completion: completion)
case .failure(let error):
DispatchQueue.main.async {
self.refreshProgress?.completeTask()
completion(.failure(error))
}
}
}
case .userDeletedZone:
DispatchQueue.main.async {
self.refreshProgress?.completeTask()
completion(.failure(CloudKitZoneError.userDeletedZone))
}
case .retry(let timeToWait):
@ -253,13 +242,11 @@ extension CloudKitZone {
default:
DispatchQueue.main.async {
self.refreshProgress?.completeTask()
completion(.failure(error!))
}
}
}
refreshProgress?.addToNumberOfTasksAndRemaining(1)
database?.add(op)
}
@ -324,7 +311,6 @@ extension CloudKitZone {
switch CloudKitZoneResult.resolve(error) {
case .success:
DispatchQueue.main.async {
self.refreshProgress?.completeTask()
self.delegate?.cloudKitDidModify(changed: changedRecords, deleted: deletedRecordKeys, completion: completion)
}
case .zoneNotFound:
@ -334,14 +320,12 @@ extension CloudKitZone {
self.fetchChangesInZone(completion: completion)
case .failure(let error):
DispatchQueue.main.async {
self.refreshProgress?.completeTask()
completion(.failure(error))
}
}
}
case .userDeletedZone:
DispatchQueue.main.async {
self.refreshProgress?.completeTask()
completion(.failure(CloudKitZoneError.userDeletedZone))
}
case .retry(let timeToWait):
@ -355,14 +339,12 @@ extension CloudKitZone {
}
default:
DispatchQueue.main.async {
self.refreshProgress?.completeTask()
completion(.failure(error!))
}
}
}
refreshProgress?.addToNumberOfTasksAndRemaining(1)
database?.add(op)
}

View File

@ -18,6 +18,8 @@ public enum LocalAccountDelegateError: String, Error {
final class LocalAccountDelegate: AccountDelegate {
private let refresher = LocalAccountRefresher()
let behaviors: AccountBehaviors = []
let isOPMLImportInProgress = false
@ -25,18 +27,16 @@ final class LocalAccountDelegate: AccountDelegate {
var credentials: Credentials?
var accountMetadata: AccountMetadata?
private let refresher = LocalAccountRefresher()
var refreshProgress: DownloadProgress {
return refresher.progress
}
let refreshProgress = DownloadProgress(numberOfTasks: 0)
func receiveRemoteNotification(for account: Account, userInfo: [AnyHashable : Any], completion: @escaping () -> Void) {
completion()
}
func refreshAll(for account: Account, completion: @escaping (Result<Void, Error>) -> Void) {
refresher.refreshFeeds(account.flattenedWebFeeds()) {
let webFeeds = account.flattenedWebFeeds()
refreshProgress.addToNumberOfTasksAndRemaining(webFeeds.count)
refresher.refreshFeeds(webFeeds, feedCompletionBlock: { _ in self.refreshProgress.completeTask() }) {
account.metadata.lastArticleFetchEndTime = Date()
completion(.success(()))
}

View File

@ -14,6 +14,7 @@ import Articles
final class LocalAccountRefresher {
private var feedCompletionBlock: ((WebFeed) -> Void)?
private var completion: (() -> Void)?
private var isSuspended = false
@ -21,11 +22,8 @@ final class LocalAccountRefresher {
return DownloadSession(delegate: self)
}()
var progress: DownloadProgress {
return downloadSession.progress
}
public func refreshFeeds(_ feeds: Set<WebFeed>, completion: @escaping () -> Void) {
public func refreshFeeds(_ feeds: Set<WebFeed>, feedCompletionBlock: @escaping (WebFeed) -> Void, completion: @escaping () -> Void) {
self.feedCompletionBlock = feedCompletionBlock
self.completion = completion
downloadSession.downloadObjects(feeds as NSSet)
}
@ -62,28 +60,37 @@ extension LocalAccountRefresher: DownloadSessionDelegate {
}
func downloadSession(_ downloadSession: DownloadSession, downloadDidCompleteForRepresentedObject representedObject: AnyObject, response: URLResponse?, data: Data, error: NSError?, completion: @escaping () -> Void) {
guard let feed = representedObject as? WebFeed, !data.isEmpty, !isSuspended else {
let feed = representedObject as! WebFeed
guard !data.isEmpty, !isSuspended else {
completion()
feedCompletionBlock?(feed)
return
}
if let error = error {
print("Error downloading \(feed.url) - \(error)")
completion()
feedCompletionBlock?(feed)
return
}
let dataHash = data.md5String
if dataHash == feed.contentHash {
completion()
feedCompletionBlock?(feed)
return
}
let parserData = ParserData(url: feed.url, data: data)
FeedParser.parse(parserData) { (parsedFeed, error) in
guard let account = feed.account, let parsedFeed = parsedFeed, error == nil else {
completion()
self.feedCompletionBlock?(feed)
return
}
account.update(feed, with: parsedFeed) { error in
if error == nil {
if let httpResponse = response as? HTTPURLResponse {
@ -93,7 +100,9 @@ extension LocalAccountRefresher: DownloadSessionDelegate {
feed.contentHash = dataHash
}
completion()
self.feedCompletionBlock?(feed)
}
}
}
@ -122,6 +131,8 @@ extension LocalAccountRefresher: DownloadSessionDelegate {
}
func downloadSession(_ downloadSession: DownloadSession, didReceiveNotModifiedResponse: URLResponse, representedObject: AnyObject) {
let feed = representedObject as! WebFeed
feedCompletionBlock?(feed)
}
func downloadSessionDidCompleteDownloadObjects(_ downloadSession: DownloadSession) {