From b7564207b38c3da2fba67a1523e2147f9ac99641 Mon Sep 17 00:00:00 2001 From: Riley Testut Date: Thu, 27 Aug 2020 16:23:50 -0700 Subject: [PATCH] Improves error handling when fetching multiple sources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fetching sources is no longer all or nothing. Now if a source cannot be fetched, it won’t prevent other sources from being updated. --- AltStore.xcodeproj/project.pbxproj | 4 + AltStore/AppDelegate.swift | 6 +- AltStore/Browse/BrowseViewController.swift | 22 +++-- AltStore/Components/ToastView.swift | 2 +- AltStore/Managing Apps/AppManager.swift | 28 ++++--- AltStore/Managing Apps/AppManagerErrors.swift | 84 +++++++++++++++++++ AltStore/Model/AppPermission.swift | 26 ++++-- AltStore/Model/Source.swift | 84 ++++++++++--------- AltStore/Model/StoreApp.swift | 73 +++++++++------- AltStore/News/NewsViewController.swift | 22 +++-- .../Operations/FetchSourceOperation.swift | 27 ++++-- 11 files changed, 262 insertions(+), 116 deletions(-) create mode 100644 AltStore/Managing Apps/AppManagerErrors.swift diff --git a/AltStore.xcodeproj/project.pbxproj b/AltStore.xcodeproj/project.pbxproj index 85e3ba6b..8b9b1f1c 100644 --- a/AltStore.xcodeproj/project.pbxproj +++ b/AltStore.xcodeproj/project.pbxproj @@ -159,6 +159,7 @@ BF770E6922BD57DD002A40FE /* Silence.m4a in Resources */ = {isa = PBXBuildFile; fileRef = BF770E6822BD57DD002A40FE /* Silence.m4a */; }; BF7C627223DBB3B400515A2D /* AltStore2ToAltStore3.xcmappingmodel in Sources */ = {isa = PBXBuildFile; fileRef = BF7C627123DBB3B400515A2D /* AltStore2ToAltStore3.xcmappingmodel */; }; BF7C627423DBB78C00515A2D /* InstalledAppPolicy.swift in Sources */ = {isa = PBXBuildFile; fileRef = BF7C627323DBB78C00515A2D /* InstalledAppPolicy.swift */; }; + BF88F97224F8727D00BB75DF /* AppManagerErrors.swift in Sources */ = {isa = PBXBuildFile; fileRef = BF88F97124F8727D00BB75DF /* AppManagerErrors.swift */; }; BF8CAE452489E772004D6CCE /* AnisetteDataManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = BF8CAE422489E772004D6CCE /* AnisetteDataManager.swift */; }; BF8CAE462489E772004D6CCE /* AppManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = BF8CAE432489E772004D6CCE /* AppManager.swift */; }; BF8CAE472489E772004D6CCE /* RequestHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = BF8CAE442489E772004D6CCE /* RequestHandler.swift */; }; @@ -525,6 +526,7 @@ BF7C627023DBB33300515A2D /* AltStore 3.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = "AltStore 3.xcdatamodel"; sourceTree = ""; }; BF7C627123DBB3B400515A2D /* AltStore2ToAltStore3.xcmappingmodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcmappingmodel; path = AltStore2ToAltStore3.xcmappingmodel; sourceTree = ""; }; BF7C627323DBB78C00515A2D /* InstalledAppPolicy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InstalledAppPolicy.swift; sourceTree = ""; }; + BF88F97124F8727D00BB75DF /* AppManagerErrors.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppManagerErrors.swift; sourceTree = ""; }; BF8CAE422489E772004D6CCE /* AnisetteDataManager.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AnisetteDataManager.swift; sourceTree = ""; }; BF8CAE432489E772004D6CCE /* AppManager.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AppManager.swift; sourceTree = ""; }; BF8CAE442489E772004D6CCE /* RequestHandler.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RequestHandler.swift; sourceTree = ""; }; @@ -1211,6 +1213,7 @@ isa = PBXGroup; children = ( BF0C4EBC22A1BD8B009A2DD7 /* AppManager.swift */, + BF88F97124F8727D00BB75DF /* AppManagerErrors.swift */, ); path = "Managing Apps"; sourceTree = ""; @@ -2069,6 +2072,7 @@ BF100C50232D7CD1006A8926 /* AltStoreToAltStore2.xcmappingmodel in Sources */, BF3D64B022E8D4B800E9056B /* AppContentViewControllerCells.swift in Sources */, BFC57A6E2416FC5D00EB891E /* InstalledAppsCollectionHeaderView.swift in Sources */, + BF88F97224F8727D00BB75DF /* AppManagerErrors.swift in Sources */, BF6C8FAE2429597900125131 /* BannerCollectionViewCell.swift in Sources */, BFBBE2DD22931B20002097FA /* AltStore.xcdatamodeld in Sources */, BF56D2AA23DF88310006506D /* AppID.swift in Sources */, diff --git a/AltStore/AppDelegate.swift b/AltStore/AppDelegate.swift index cf3cae3a..47f200b8 100644 --- a/AltStore/AppDelegate.swift +++ b/AltStore/AppDelegate.swift @@ -302,13 +302,11 @@ private extension AppDelegate dispatchGroup.enter() AppManager.shared.fetchSources() { (result) in - fetchSourcesResult = result + fetchSourcesResult = result.map { $0.0 }.mapError { $0 as Error } do { - let sources = try result.get() - - guard let context = sources.first?.managedObjectContext else { return } + let (_, context) = try result.get() let previousUpdatesFetchRequest = InstalledApp.updatesFetchRequest() as! NSFetchRequest previousUpdatesFetchRequest.includesPendingChanges = false diff --git a/AltStore/Browse/BrowseViewController.swift b/AltStore/Browse/BrowseViewController.swift index d104b261..356e0309 100644 --- a/AltStore/Browse/BrowseViewController.swift +++ b/AltStore/Browse/BrowseViewController.swift @@ -178,20 +178,26 @@ private extension BrowseViewController AppManager.shared.fetchSources() { (result) in do { - let sources = try result.get() - try sources.first?.managedObjectContext?.save() - - DispatchQueue.main.async { - self.loadingState = .finished(.success(())) + do + { + let (_, context) = try result.get() + try context.save() + + DispatchQueue.main.async { + self.loadingState = .finished(.success(())) + } + } + catch let error as AppManager.FetchSourcesError + { + try error.managedObjectContext?.save() + throw error } } - catch let error as NSError + catch { DispatchQueue.main.async { if self.dataSource.itemCount > 0 { - let error = error.withLocalizedFailure(NSLocalizedString("Failed to Fetch Sources", comment: "")) - let toastView = ToastView(error: error) toastView.show(in: self) } diff --git a/AltStore/Components/ToastView.swift b/AltStore/Components/ToastView.swift index 8fc81407..ecee87e8 100644 --- a/AltStore/Components/ToastView.swift +++ b/AltStore/Components/ToastView.swift @@ -77,7 +77,7 @@ class ToastView: RSTToastView else { text = error.localizedDescription - detailText = underlyingError?.localizedDescription + detailText = underlyingError?.localizedDescription ?? error.localizedRecoverySuggestion } self.init(text: text, detailText: detailText) diff --git a/AltStore/Managing Apps/AppManager.swift b/AltStore/Managing Apps/AppManager.swift index 2600f3d6..b922681d 100644 --- a/AltStore/Managing Apps/AppManager.swift +++ b/AltStore/Managing Apps/AppManager.swift @@ -197,15 +197,16 @@ extension AppManager self.run([fetchSourceOperation], context: nil) } - func fetchSources(completionHandler: @escaping (Result, Error>) -> Void) + func fetchSources(completionHandler: @escaping (Result<(Set, NSManagedObjectContext), FetchSourcesError>) -> Void) { DatabaseManager.shared.persistentContainer.performBackgroundTask { (context) in let sources = Source.all(in: context) - guard !sources.isEmpty else { return completionHandler(.failure(OperationError.noSources)) } + guard !sources.isEmpty else { return completionHandler(.failure(.init(OperationError.noSources))) } let dispatchGroup = DispatchGroup() var fetchedSources = Set() - var error: Error? + + var errors = [Source: Error]() let managedObjectContext = DatabaseManager.shared.persistentContainer.newBackgroundContext() @@ -216,8 +217,10 @@ extension AppManager fetchSourceOperation.resultHandler = { (result) in switch result { - case .failure(let e): error = e case .success(let source): fetchedSources.insert(source) + case .failure(let error): + let source = managedObjectContext.object(with: source.objectID) as! Source + errors[source] = error } dispatchGroup.leave() @@ -227,14 +230,15 @@ extension AppManager } dispatchGroup.notify(queue: .global()) { - if let error = error - { - completionHandler(.failure(error)) - } - else - { - managedObjectContext.perform { - completionHandler(.success(fetchedSources)) + managedObjectContext.perform { + if !errors.isEmpty + { + let sources = Set(sources.compactMap { managedObjectContext.object(with: $0.objectID) as? Source }) + completionHandler(.failure(.init(sources: sources, errors: errors, context: managedObjectContext))) + } + else + { + completionHandler(.success((fetchedSources, managedObjectContext))) } } diff --git a/AltStore/Managing Apps/AppManagerErrors.swift b/AltStore/Managing Apps/AppManagerErrors.swift new file mode 100644 index 00000000..c36501bd --- /dev/null +++ b/AltStore/Managing Apps/AppManagerErrors.swift @@ -0,0 +1,84 @@ +// +// AppManagerErrors.swift +// AltStore +// +// Created by Riley Testut on 8/27/20. +// Copyright © 2020 Riley Testut. All rights reserved. +// + +import Foundation +import CoreData + +extension AppManager +{ + struct FetchSourcesError: LocalizedError, CustomNSError + { + var primaryError: Error? + + var sources: Set? + var errors = [Source: Error]() + + var managedObjectContext: NSManagedObjectContext? + + var errorDescription: String? { + if let error = self.primaryError + { + return error.localizedDescription + } + else + { + var localizedDescription: String? + + self.managedObjectContext?.performAndWait { + if self.sources?.count == 1 + { + localizedDescription = NSLocalizedString("Could not refresh store.", comment: "") + } + else if self.errors.count == 1 + { + guard let source = self.errors.keys.first else { return } + localizedDescription = String(format: NSLocalizedString("Could not refresh source “%@”.", comment: ""), source.name) + } + else + { + localizedDescription = String(format: NSLocalizedString("Could not refresh %@ sources.", comment: ""), NSNumber(value: self.errors.count)) + } + } + + return localizedDescription + } + } + + var recoverySuggestion: String? { + if let error = self.primaryError as NSError? + { + return error.localizedRecoverySuggestion + } + else if self.errors.count == 1 + { + return nil + } + else + { + return NSLocalizedString("Tap to view source errors.", comment: "") + } + } + + var errorUserInfo: [String : Any] { + guard let error = self.errors.values.first, self.errors.count == 1 else { return [:] } + return [NSUnderlyingErrorKey: error] + } + + init(_ error: Error) + { + self.primaryError = error + } + + init(sources: Set, errors: [Source: Error], context: NSManagedObjectContext) + { + self.sources = sources + self.errors = errors + self.managedObjectContext = context + } + } +} diff --git a/AltStore/Model/AppPermission.swift b/AltStore/Model/AppPermission.swift index 507ed25b..2e1d0d89 100644 --- a/AltStore/Model/AppPermission.swift +++ b/AltStore/Model/AppPermission.swift @@ -67,15 +67,25 @@ class AppPermission: NSManagedObject, Decodable, Fetchable { guard let context = decoder.managedObjectContext else { preconditionFailure("Decoder must have non-nil NSManagedObjectContext.") } - super.init(entity: AppPermission.entity(), insertInto: nil) + super.init(entity: AppPermission.entity(), insertInto: context) - let container = try decoder.container(keyedBy: CodingKeys.self) - self.usageDescription = try container.decode(String.self, forKey: .usageDescription) - - let rawType = try container.decode(String.self, forKey: .type) - self.type = ALTAppPermissionType(rawValue: rawType) - - context.insert(self) + do + { + let container = try decoder.container(keyedBy: CodingKeys.self) + self.usageDescription = try container.decode(String.self, forKey: .usageDescription) + + let rawType = try container.decode(String.self, forKey: .type) + self.type = ALTAppPermissionType(rawValue: rawType) + } + catch + { + if let context = self.managedObjectContext + { + context.delete(self) + } + + throw error + } } } diff --git a/AltStore/Model/Source.swift b/AltStore/Model/Source.swift index 34ef5d7e..8e4bf2b8 100644 --- a/AltStore/Model/Source.swift +++ b/AltStore/Model/Source.swift @@ -88,52 +88,60 @@ class Source: NSManagedObject, Fetchable, Decodable guard let context = decoder.managedObjectContext else { preconditionFailure("Decoder must have non-nil NSManagedObjectContext.") } guard let sourceURL = decoder.sourceURL else { preconditionFailure("Decoder must have non-nil sourceURL.") } - super.init(entity: Source.entity(), insertInto: nil) + super.init(entity: Source.entity(), insertInto: context) - self.sourceURL = sourceURL - - let container = try decoder.container(keyedBy: CodingKeys.self) - self.name = try container.decode(String.self, forKey: .name) - self.identifier = try container.decode(String.self, forKey: .identifier) - - let userInfo = try container.decodeIfPresent([String: String].self, forKey: .userInfo) - self.userInfo = userInfo?.reduce(into: [:]) { $0[ALTSourceUserInfoKey($1.key)] = $1.value } - - let apps = try container.decodeIfPresent([StoreApp].self, forKey: .apps) ?? [] - let appsByID = Dictionary(apps.map { ($0.bundleIdentifier, $0) }, uniquingKeysWith: { (a, b) in return a }) - - for (index, app) in apps.enumerated() + do { - app.sourceIdentifier = self.identifier - app.sortIndex = Int32(index) - } - - let newsItems = try container.decodeIfPresent([NewsItem].self, forKey: .news) ?? [] - for (index, item) in newsItems.enumerated() - { - item.sourceIdentifier = self.identifier - item.sortIndex = Int32(index) - } - - context.insert(self) - - for newsItem in newsItems - { - guard let appID = newsItem.appID else { continue } + self.sourceURL = sourceURL - if let storeApp = appsByID[appID] + let container = try decoder.container(keyedBy: CodingKeys.self) + self.name = try container.decode(String.self, forKey: .name) + self.identifier = try container.decode(String.self, forKey: .identifier) + + let userInfo = try container.decodeIfPresent([String: String].self, forKey: .userInfo) + self.userInfo = userInfo?.reduce(into: [:]) { $0[ALTSourceUserInfoKey($1.key)] = $1.value } + + let apps = try container.decodeIfPresent([StoreApp].self, forKey: .apps) ?? [] + let appsByID = Dictionary(apps.map { ($0.bundleIdentifier, $0) }, uniquingKeysWith: { (a, b) in return a }) + + for (index, app) in apps.enumerated() { - newsItem.storeApp = storeApp + app.sourceIdentifier = self.identifier + app.sortIndex = Int32(index) } - else + self._apps = NSMutableOrderedSet(array: apps) + + let newsItems = try container.decodeIfPresent([NewsItem].self, forKey: .news) ?? [] + for (index, item) in newsItems.enumerated() { - newsItem.storeApp = nil + item.sourceIdentifier = self.identifier + item.sortIndex = Int32(index) } + + for newsItem in newsItems + { + guard let appID = newsItem.appID else { continue } + + if let storeApp = appsByID[appID] + { + newsItem.storeApp = storeApp + } + else + { + newsItem.storeApp = nil + } + } + self._newsItems = NSMutableOrderedSet(array: newsItems) + } + catch + { + if let context = self.managedObjectContext + { + context.delete(self) + } + + throw error } - - // Must assign after we're inserted into context. - self._apps = NSMutableOrderedSet(array: apps) - self._newsItems = NSMutableOrderedSet(array: newsItems) } } diff --git a/AltStore/Model/StoreApp.swift b/AltStore/Model/StoreApp.swift index d6a7abe2..b25d80dc 100644 --- a/AltStore/Model/StoreApp.swift +++ b/AltStore/Model/StoreApp.swift @@ -104,43 +104,52 @@ class StoreApp: NSManagedObject, Decodable, Fetchable { guard let context = decoder.managedObjectContext else { preconditionFailure("Decoder must have non-nil NSManagedObjectContext.") } - super.init(entity: StoreApp.entity(), insertInto: nil) + // Must initialize with context in order for child context saves to work correctly. + super.init(entity: StoreApp.entity(), insertInto: context) - let container = try decoder.container(keyedBy: CodingKeys.self) - self.name = try container.decode(String.self, forKey: .name) - self.bundleIdentifier = try container.decode(String.self, forKey: .bundleIdentifier) - self.developerName = try container.decode(String.self, forKey: .developerName) - self.localizedDescription = try container.decode(String.self, forKey: .localizedDescription) - - self.subtitle = try container.decodeIfPresent(String.self, forKey: .subtitle) - - self.version = try container.decode(String.self, forKey: .version) - self.versionDate = try container.decode(Date.self, forKey: .versionDate) - self.versionDescription = try container.decodeIfPresent(String.self, forKey: .versionDescription) - - self.iconURL = try container.decode(URL.self, forKey: .iconURL) - self.screenshotURLs = try container.decodeIfPresent([URL].self, forKey: .screenshotURLs) ?? [] - - self.downloadURL = try container.decode(URL.self, forKey: .downloadURL) - - if let tintColorHex = try container.decodeIfPresent(String.self, forKey: .tintColor) + do { - guard let tintColor = UIColor(hexString: tintColorHex) else { - throw DecodingError.dataCorruptedError(forKey: .tintColor, in: container, debugDescription: "Hex code is invalid.") + let container = try decoder.container(keyedBy: CodingKeys.self) + self.name = try container.decode(String.self, forKey: .name) + self.bundleIdentifier = try container.decode(String.self, forKey: .bundleIdentifier) + self.developerName = try container.decode(String.self, forKey: .developerName) + self.localizedDescription = try container.decode(String.self, forKey: .localizedDescription) + + self.subtitle = try container.decodeIfPresent(String.self, forKey: .subtitle) + + self.version = try container.decode(String.self, forKey: .version) + self.versionDate = try container.decode(Date.self, forKey: .versionDate) + self.versionDescription = try container.decodeIfPresent(String.self, forKey: .versionDescription) + + self.iconURL = try container.decode(URL.self, forKey: .iconURL) + self.screenshotURLs = try container.decodeIfPresent([URL].self, forKey: .screenshotURLs) ?? [] + + self.downloadURL = try container.decode(URL.self, forKey: .downloadURL) + + if let tintColorHex = try container.decodeIfPresent(String.self, forKey: .tintColor) + { + guard let tintColor = UIColor(hexString: tintColorHex) else { + throw DecodingError.dataCorruptedError(forKey: .tintColor, in: container, debugDescription: "Hex code is invalid.") + } + + self.tintColor = tintColor } - self.tintColor = tintColor + self.size = try container.decode(Int32.self, forKey: .size) + self.isBeta = try container.decodeIfPresent(Bool.self, forKey: .isBeta) ?? false + + let permissions = try container.decodeIfPresent([AppPermission].self, forKey: .permissions) ?? [] + self._permissions = NSOrderedSet(array: permissions) + } + catch + { + if let context = self.managedObjectContext + { + context.delete(self) + } + + throw error } - - self.size = try container.decode(Int32.self, forKey: .size) - self.isBeta = try container.decodeIfPresent(Bool.self, forKey: .isBeta) ?? false - - let permissions = try container.decodeIfPresent([AppPermission].self, forKey: .permissions) ?? [] - - context.insert(self) - - // Must assign after we're inserted into context. - self._permissions = NSOrderedSet(array: permissions) } } diff --git a/AltStore/News/NewsViewController.swift b/AltStore/News/NewsViewController.swift index ba659c61..5c7eb3e5 100644 --- a/AltStore/News/NewsViewController.swift +++ b/AltStore/News/NewsViewController.swift @@ -177,20 +177,26 @@ private extension NewsViewController AppManager.shared.fetchSources() { (result) in do { - let sources = try result.get() - try sources.first?.managedObjectContext?.save() - - DispatchQueue.main.async { - self.loadingState = .finished(.success(())) + do + { + let (_, context) = try result.get() + try context.save() + + DispatchQueue.main.async { + self.loadingState = .finished(.success(())) + } + } + catch let error as AppManager.FetchSourcesError + { + try error.managedObjectContext?.save() + throw error } } - catch let error as NSError + catch { DispatchQueue.main.async { if self.dataSource.itemCount > 0 { - let error = error.withLocalizedFailure(NSLocalizedString("Failed to Fetch Sources", comment: "")) - let toastView = ToastView(error: error) toastView.show(in: self) } diff --git a/AltStore/Operations/FetchSourceOperation.swift b/AltStore/Operations/FetchSourceOperation.swift index 8d823db0..be47a60c 100644 --- a/AltStore/Operations/FetchSourceOperation.swift +++ b/AltStore/Operations/FetchSourceOperation.swift @@ -41,7 +41,10 @@ class FetchSourceOperation: ResultOperation super.main() let dataTask = self.session.dataTask(with: self.sourceURL) { (data, response, error) in - self.managedObjectContext.perform { + + let childContext = DatabaseManager.shared.persistentContainer.newBackgroundContext(withParent: self.managedObjectContext) + childContext.mergePolicy = NSOverwriteMergePolicy + childContext.perform { do { let (data, _) = try Result((data, response), error).get() @@ -68,21 +71,35 @@ class FetchSourceOperation: ResultOperation throw DecodingError.dataCorruptedError(in: container, debugDescription: "Date is in invalid format.") }) - decoder.managedObjectContext = self.managedObjectContext + decoder.managedObjectContext = childContext decoder.sourceURL = self.sourceURL let source = try decoder.decode(Source.self, from: data) + let identifier = source.identifier - if source.identifier == Source.altStoreIdentifier, let patreonAccessToken = source.userInfo?[.patreonAccessToken] + if identifier == Source.altStoreIdentifier, let patreonAccessToken = source.userInfo?[.patreonAccessToken] { Keychain.shared.patreonCreatorAccessToken = patreonAccessToken } - self.finish(.success(source)) + try childContext.save() + + self.managedObjectContext.perform { + if let source = Source.first(satisfying: NSPredicate(format: "%K == %@", #keyPath(Source.identifier), identifier), in: self.managedObjectContext) + { + self.finish(.success(source)) + } + else + { + self.finish(.failure(OperationError.noSources)) + } + } } catch { - self.finish(.failure(error)) + self.managedObjectContext.perform { + self.finish(.failure(error)) + } } } }