refactor(ios/engine): pkg-based resource updates

This commit is contained in:
jahorton 2020-07-20 10:36:02 +07:00
parent d7955faa8c
commit 36edc9cd44
5 changed files with 147 additions and 145 deletions

View file

@ -144,7 +144,7 @@ class LanguageLMDetailViewController: UITableViewController, UIAlertViewDelegate
let package = packages[lexicalModelIndex]
let lmFullID = package.0.fullID
let completionClosure: ResourceDownloadManager.CompletionHandler<LexicalModelKeymanPackage> = { package, error in
ResourceDownloadManager.shared.standardLexicalModelInstallCompletionBlock(forFullID: lmFullID)(package, error)
try? ResourceDownloadManager.shared.standardLexicalModelInstallCompletionBlock(forFullID: lmFullID)(package, error)
if let lm = package?.findResource(withID: lmFullID) {
self.onSuccessClosure?(lm)

View file

@ -38,9 +38,8 @@ public struct PackageDownloadFailedNotification {
public typealias BatchUpdateStartedNotification = [AnyLanguageResource]
public struct BatchUpdateCompletedNotification {
public let successes: [[AnyLanguageResource]]
public let failures: [[AnyLanguageResource]]
public let errors: [Error]
public let successes: [KeymanPackage.Key]
public let failures: [(KeymanPackage.Key, Error)]
}
public typealias KeyboardLoadedNotification = InstallableKeyboard

View file

@ -22,7 +22,7 @@ public class ResourceDownloadManager {
public static let DISTRIBUTION_CACHE_VALIDITY_THRESHOLD = TimeInterval(60*24*7) // in seconds. 60 minutes, 24 hrs, 7 days.
public typealias CompletionHandler<Package: KeymanPackage> = (Package?, Error?) -> Void
public typealias CompletionHandler<Package: KeymanPackage> = (Package?, Error?) throws -> Void
public typealias BatchCompletionHandler = () -> Void
internal typealias InternalBatchCompletionHandler = (CompositeBatch) -> Void
@ -54,7 +54,7 @@ public class ResourceDownloadManager {
guard let result = result, error == nil else {
log.info("Error occurred requesting location for \(fullID.description)")
self.resourceDownloadFailed(withKey: packageKey, with: error ?? .noData)
completionBlock?(nil, error ?? .noData)
try? completionBlock?(nil, error ?? .noData)
return
}
@ -65,7 +65,7 @@ public class ResourceDownloadManager {
}
}
self.resourceDownloadFailed(withKey: packageKey, with: Queries.ResultError.unqueried)
completionBlock?(nil, Queries.ResultError.unqueried)
try? completionBlock?(nil, Queries.ResultError.unqueried)
return
}
@ -74,7 +74,7 @@ public class ResourceDownloadManager {
let err = self.downloader.state.error ??
NSError(domain: "Keyman", code: 0, userInfo: [NSLocalizedDescriptionKey: "Already busy downloading something"])
self.resourceDownloadFailed(withKey: packageKey, with: err)
completionBlock?(nil, err)
try? completionBlock?(nil, err)
return
}
@ -384,71 +384,68 @@ public class ResourceDownloadManager {
}
}
// Possibly worth not deprecating, as its continued existence doesn't exactly hurt anything.
// But its intended partner method will no longer exist, so... yeah.
@available(*, deprecated, message: "Deprecated in favor of `getKeysForUpdatablePackages`.")
public func getAvailableUpdates() -> [AnyLanguageResource]? {
let updatablePackages = getKeysForUpdatablePackages()
public func performBatchUpdate(forPackageKeys keysToUpdate: Set<KeymanPackage.Key>? = nil,
withNotifications: Bool = true,
completionBlock: (([KeymanPackage.Key], [(KeymanPackage.Key, Error)]) -> Void)? = nil) {
let engineUpdatables = getKeysForUpdatablePackages()
var updatables = Set<KeymanPackage.Key>()
// To maintain the original behavior.
guard updatablePackages.count > 0 else {
return nil
}
var invalids: [(KeymanPackage.Key, Error)] = []
// As updatablePackages is a Set (and keys are Hashable), the lookups are O(1).
let updatables: [AnyLanguageResource] = Storage.active.userDefaults.userResources?.compactMap { updatablePackages.contains($0.packageKey) ? $0 : nil } ?? []
if updatables.count > 0 {
return updatables
if keysToUpdate == nil {
updatables = engineUpdatables
} else {
return nil
}
}
// Verify that KeymanEngine actually can update the entry.
updatables = engineUpdatables.union(keysToUpdate!)
@available(*, deprecated, message: "") // TODO: Properly document once the target method is written.
public func performUpdates(forResources resources: [AnyLanguageResource]) {
// The plan is to create new notifications to handle batch updates here, rather than
// require a UI to manage the update queue.
var batches: [AnyDownloadBatch] = []
/* For fixing the TODOs: we can probably map the resources to a Set of their
* represented package keys, then just use the package-key based update method.
*
* You know, once it's written.
*/
// TODO: All package updates are currently broken, as apiKeyboardRepository will specify the wrong file.
// TODO: Merge the keyboard and lexical model pathways; it's WET code.
resources.forEach { res in
if let kbd = res as? InstallableKeyboard {
// if let filename = Manager.shared.apiKeyboardRepository.keyboards?[kbd.id]?.filename,
// let path = URL.init(string: filename) {
// let batch = self.buildPackageBatch(forFullID: kbd.fullID, from: path, withResource: kbd) { package, error in
// if let package = package {
// try? ResourceFileManager.shared.install(resourceWithID: kbd.fullID, from: package)
// }
// // else error: already handled by wrapping closure set within buildPackageBatch.
// }
// batches.append(batch)
// }
} else if let lex = res as? InstallableLexicalModel {
// if let filename = Manager.shared.apiLexicalModelRepository.lexicalModels?[lex.id]?.packageFilename,
// let path = URL.init(string: filename) {
// let batch = self.buildPackageBatch(withKey: lex.packageKey, from: path, withResource: lex) { package, error in
// if let package = package {
// try? ResourceFileManager.shared.install(resourceWithID: lex.fullID, from: package)
// }
// // else error: already handled by wrapping closure set within buildPackageBatch.
// }
// batches.append(batch)
// }
// Generate errors for any entries that KeymanEngine cannot update.
keysToUpdate!.forEach { key in
if !updatables.contains(key) {
// TODO: Better error definition
invalids.append( (key, NSError()) )
}
}
}
let batchUpdate = CompositeBatch(queue: batches.map { return DownloadNode.simpleBatch($0) },
startBlock: resourceBatchUpdateStartClosure(for: resources),
completionBlock: resourceBatchUpdateCompletionClosure(for: resources))
downloader.queue(.compositeBatch(batchUpdate))
var updateMapping: Dictionary<KeymanPackage.Key, AnyDownloadBatch> = [:]
updatables.forEach { key in
guard let downloadURL = Storage.active.userDefaults.cachedPackageQueryResult(forPackageKey: key)!.downloadURL else {
// Note error - download URL missing. Shouldn't be possible, but still.
// TODO: Better error definition
invalids.append( (key, NSError()) )
return
}
updateMapping[key] = self.buildPackageBatch(withKey: key,
from: downloadURL) { package, error in
guard let package = package, error == nil else {
let errString = error != nil ? String(describing: error!) : ""
log.error("Could not successfully download package \(key) for update: \(errString)")
return
}
if let kbdPackage = package as? KeyboardKeymanPackage {
let updatables = ResourceFileManager.shared.findPotentialUpdates(in: kbdPackage)
try ResourceFileManager.shared.install(resourcesWithIDs: updatables.map { $0.fullID }, from: kbdPackage)
} else if let lmPackage = package as? LexicalModelKeymanPackage {
let updatables = ResourceFileManager.shared.findPotentialUpdates(in: lmPackage)
try ResourceFileManager.shared.install(resourcesWithIDs: updatables.map { $0.fullID }, from: lmPackage)
}
}
}
// Build the composite batch.
let batchNodes: [DownloadNode] = updateMapping.values.map { .simpleBatch($0) }
let updateStartClosure: (() -> Void)? = nil
let updateCompletionClosure: InternalBatchCompletionHandler = resourceBatchUpdateCompletionClosure(withNotifications: withNotifications, completionBlock: completionBlock)
let updateBatch = CompositeBatch(queue: batchNodes, startBlock: updateStartClosure, completionBlock: updateCompletionClosure)
downloader.queue(.compositeBatch(updateBatch))
}
static func packageKeys(forResources resources: [AnyLanguageResource]) -> Set<KeymanPackage.Key> {
let keys = resources.map { $0.packageKey }
return Set(keys)
}
@available(*, deprecated)
@ -484,7 +481,11 @@ public class ResourceDownloadManager {
self.resourceDownloadCompleted(with: package)
}
handler?(package, error)
do {
try handler?(package, error)
} catch {
log.error("Unhandled error occurred after resource successfully downloaded: \(String(describing: error))")
}
// After the custom handler operates, ensure that any changes it made are synchronized for use
// with the app extension, too.
@ -503,30 +504,32 @@ public class ResourceDownloadManager {
}
}
internal func resourceBatchUpdateCompletionClosure(for resources: [AnyLanguageResource],
completionBlock: BatchCompletionHandler? = nil)
internal func resourceBatchUpdateCompletionClosure(withNotifications: Bool,
completionBlock: (([KeymanPackage.Key], [(KeymanPackage.Key, Error)]) -> Void)? = nil)
-> InternalBatchCompletionHandler {
return { batch in
var successes: [KeymanPackage.Key] = []
var failures: [KeymanPackage.Key] = []
var errors: [Error] = []
var failures: [(KeymanPackage.Key, Error)] = []
// Remember, since this is for .composite batches, batch.tasks is of type [DownloadBatch].
for (index, _) in batch.tasks.enumerated() {
if batch.errors[index] == nil {
successes.append(contentsOf: batch.batches[index].packageKeys)
} else {
failures.append(contentsOf: batch.batches[index].packageKeys)
errors.append(batch.errors[index]!)
batch.batchQueue.forEach{ tuple in
if case let .simpleBatch(node) = tuple.0 {
if let err = node.errors.first(where: { $0 != nil }), let error = err {
failures.append( (node.packageKey, error) )
} else {
successes.append(node.packageKey)
}
}
}
// TODO: Rework batch update notifications to return package keys.
let notification = BatchUpdateCompletedNotification(successes: [], failures: [], errors: errors)
NotificationCenter.default.post(name: Notifications.batchUpdateCompleted,
object: self,
value: notification)
completionBlock?()
if withNotifications {
// Send update notification to the UI.
let notification = BatchUpdateCompletedNotification(successes: successes, failures: failures)
NotificationCenter.default.post(name: Notifications.batchUpdateCompleted,
object: self,
value: notification)
}
completionBlock?(successes, failures)
}
}

View file

@ -28,16 +28,7 @@ enum DownloadNode {
case .simpleBatch(let batch):
return batch.errors
case .compositeBatch(let node):
return node.errors
}
}
set(value) {
switch(self) {
case .simpleBatch(var batch):
batch.errors = value
case .compositeBatch(let node):
node.errors = value
return node.batchQueue.map{ $0.1 }
}
}
}
@ -50,7 +41,6 @@ protocol AnyDownloadTask {
}
class DownloadTask: AnyDownloadTask {
public final var packageKey: KeymanPackage.Key?
public final var request: HTTPDownloadRequest
public var file: URL {
if let file = finalFile {
@ -66,13 +56,11 @@ class DownloadTask: AnyDownloadTask {
public init(do request: HTTPDownloadRequest, forPackage packageKey: KeymanPackage.Key?) {
self.request = request
self.packageKey = packageKey
}
public init(forPackageWithKey packageKey: KeymanPackage.Key,
from url: URL,
as destURL: URL, tempURL: URL) {
self.packageKey = packageKey
let request = HTTPDownloadRequest(url: url, userInfo: [:])
request.destinationFile = tempURL.path
@ -110,14 +98,17 @@ enum DownloadActivityType {
protocol AnyDownloadBatch {
var tasks: [AnyDownloadTask] { get }
var packageKey: KeymanPackage.Key { get }
// Needed for DownloadNode compliance.
var packageKeys: [KeymanPackage.Key] { get }
var startBlock: (() -> Void)? { get }
var errors: [Error?] { get set }
func completeWithCancellation() -> Void
func completeWithError(error: Error) -> Void
func completeWithPackage(fromKMP file: URL) -> Void
func completeWithCancellation() throws -> Void
func completeWithError(error: Error) throws -> Void
func completeWithPackage(fromKMP file: URL) throws -> Void
}
/**
@ -127,25 +118,17 @@ class DownloadBatch<Package: KeymanPackage>: AnyDownloadBatch {
typealias CompletionHandler = ResourceDownloadManager.CompletionHandler
public final var downloadTasks: [DownloadTask]
public let packageKey: KeymanPackage.Key
var errors: [Error?] // Only used by the ResourceDownloadQueue.
public final var startBlock: (() -> Void)? = nil
public final var completionBlock: CompletionHandler<Package>? = nil
public init?(do tasks: [DownloadTask],
startBlock: (() -> Void)? = nil,
completionBlock: CompletionHandler<Package>? = nil) {
self.downloadTasks = tasks
self.errors = Array(repeating: nil, count: tasks.count)
self.startBlock = startBlock
self.completionBlock = completionBlock
}
public init(forPackageWithKey packageKey: KeymanPackage.Key,
from url: URL,
startBlock: (() -> Void)?,
completionBlock: CompletionHandler<Package>?) {
// If we can't build a proper DownloadTask, we can't build the batch.
self.packageKey = packageKey
let tempArtifact = ResourceFileManager.shared.packageDownloadTempPath(forKey: packageKey)
let finalFile = ResourceFileManager.shared.cachedPackagePath(forKey: packageKey)
@ -168,60 +151,57 @@ class DownloadBatch<Package: KeymanPackage>: AnyDownloadBatch {
public var packageKeys: [KeymanPackage.Key] {
get {
return downloadTasks.compactMap { $0.packageKey }
return [ packageKey ]
}
}
public func completeWithCancellation() {
public func completeWithCancellation() throws {
let complete = completionBlock
completionBlock = nil
complete?(nil, nil)
try complete?(nil, nil)
}
public func completeWithError(error: Error) {
public func completeWithError(error: Error) throws {
let complete = completionBlock
completionBlock = nil
complete?(nil, error)
try complete?(nil, error)
}
public func completeWithPackage(fromKMP file: URL) {
public func completeWithPackage(fromKMP file: URL) throws {
let complete = completionBlock
completionBlock = nil
do {
if let package = try ResourceFileManager.shared.prepareKMPInstall(from: file) as? Package {
complete?(package, nil)
try complete?(package, nil)
} else {
complete?(nil, KMPError.invalidPackage)
try complete?(nil, KMPError.invalidPackage)
}
} catch {
complete?(nil, error)
try complete?(nil, error)
}
}
}
class CompositeBatch {
public final var batches: [DownloadNode]
var errors: [Error?] // Only used by the ResourceDownloadQueue.
public final var batchQueue: [(DownloadNode, Error?)]
public final var startBlock: (() -> Void)? = nil
public final var completionBlock: ResourceDownloadManager.InternalBatchCompletionHandler? = nil
public init(queue: [DownloadNode],
startBlock: (() -> Void)? = nil,
completionBlock: ResourceDownloadManager.InternalBatchCompletionHandler? = nil) {
self.batches = queue
self.errors = Array(repeating: nil, count: batches.count)
self.batchQueue = queue.map { ($0, nil) }
self.startBlock = startBlock
self.completionBlock = completionBlock
}
public var tasks: [DownloadNode] {
return batches
return batchQueue.map { $0.0 }
}
public var packageKeys: [KeymanPackage.Key] {
return batches.flatMap { $0.packageKeys }
return batchQueue.flatMap { $0.0.packageKeys }
}
}
@ -336,7 +316,7 @@ class ResourceDownloadQueue: HTTPDownloadDelegate {
}
}
private func finalizeCurrentBatch() {
private func finalizeCurrentBatch(withError error: Error? = nil) {
let frame = queueStack[queueStack.count - 1]
if queueStack.count == 1 {
@ -347,6 +327,9 @@ class ResourceDownloadQueue: HTTPDownloadDelegate {
// when everything's done instead. Might be worth a thought.
frame.nodes.remove(at: 0)
} else {
if case let .compositeBatch(batch) = frame.batch {
batch.batchQueue[frame.index].1 = error
}
// Batches in subframes should be kept so that we can report progress; increment the index instead.
frame.index += 1
}
@ -368,14 +351,16 @@ class ResourceDownloadQueue: HTTPDownloadDelegate {
if frame.index == frame.nodes.count {
// We've hit the end of this stack frame's commands; time to pop and continue from the previous frame's perspective.
_ = queueStack.popLast()
// Of course, this means we've "finished" a batch download. We can use the same handlers as before.
finalizeCurrentBatch()
// Of course, this means we've "finished" a batch download. We can use the same handlers as before.
// if-check below: "if the current stack-frame came from a composite batch, let node = that 'composite batch'"
if case .compositeBatch(let node) = frame.batch {
// The base handler requires access to the batch's tracked success/failure data.
let error = node.batchQueue.first(where: { $0.1 != nil })?.1
finalizeCurrentBatch(withError: error)
node.completionBlock?(node)
} else {
fatalError("Unexpected download queue state; cannot recover")
}
if autoExecute {
@ -456,11 +441,14 @@ class ResourceDownloadQueue: HTTPDownloadDelegate {
let batch = queue.userInfo[Key.downloadBatch] as! AnyDownloadBatch
let packagePath = batch.tasks[0].file
batch.completeWithPackage(fromKMP: packagePath)
// Completing the queue means having completed a batch. We should only move forward in this class's
// queue at this time, once a batch's task queue is complete.
finalizeCurrentBatch()
do {
try batch.completeWithPackage(fromKMP: packagePath)
// Completing the queue means having completed a batch. We should only move forward in this class's
// queue at this time, once a batch's task queue is complete.
finalizeCurrentBatch()
} catch {
finalizeCurrentBatch(withError: error)
}
if autoExecute {
executeNext()
@ -469,7 +457,7 @@ class ResourceDownloadQueue: HTTPDownloadDelegate {
func downloadQueueCancelled(_ queue: HTTPDownloader) {
if case .simpleBatch(let batch) = self.currentBatch {
batch.completeWithCancellation()
try? batch.completeWithCancellation()
}
// In case we're part of a 'composite' operation, we should still keep the queue moving.
@ -495,7 +483,10 @@ class ResourceDownloadQueue: HTTPDownloadDelegate {
let errorMessage = "\(request.responseStatusMessage ?? ""): \(request.url)"
let error = NSError(domain: "Keyman", code: 0,
userInfo: [NSLocalizedDescriptionKey: errorMessage])
currentFrame.batch?.errors[currentFrame.index] = error
if case var .simpleBatch(batch) = currentFrame.batch {
batch.errors[currentFrame.index] = error
}
// Now that we've synthesized an appropriate error instance, use the same handler
// as for HTTPDownloader's 'failed' condition.
@ -513,9 +504,9 @@ class ResourceDownloadQueue: HTTPDownloadDelegate {
}
func downloadRequestFailed(_ request: HTTPDownloadRequest, with error: Error?) {
currentFrame.batch?.errors[currentFrame.index] = error
let task = request.userInfo[Key.downloadTask] as! AnyDownloadTask
let batch = request.userInfo[Key.downloadBatch] as! AnyDownloadBatch
var batch = request.userInfo[Key.downloadBatch] as! AnyDownloadBatch
batch.errors[currentFrame.index] = error
var err: Error
if let error = error {
@ -526,7 +517,7 @@ class ResourceDownloadQueue: HTTPDownloadDelegate {
}
try? task.downloadFinalizationBlock?(false)
batch.completeWithError(error: err)
try? batch.completeWithError(error: err)
downloader!.cancelAllOperations()
}
}

View file

@ -130,8 +130,13 @@ public class InstalledLanguagesViewController: UITableViewController, UIAlertVie
// Do the actual updates!
// TODO: Consider prompting per resource, rather than wholesale as a group.
// (This would be an enhancement, though.)
let availableUpdates = ResourceDownloadManager.shared.getAvailableUpdates()!
ResourceDownloadManager.shared.performUpdates(forResources: availableUpdates)
let availableUpdates = ResourceDownloadManager.shared.getKeysForUpdatablePackages()
ResourceDownloadManager.shared.performBatchUpdate(forPackageKeys: availableUpdates,
withNotifications: true,
completionBlock: { successes, failures in
// TODO: future feature: consider reworking the notification listener for use with this callback.
// (See `batchUpdateCompleted` later in this file.)
})
}
override public func numberOfSections(in tableView: UITableView) -> Int {
@ -378,10 +383,14 @@ public class InstalledLanguagesViewController: UITableViewController, UIAlertVie
toolbar.displayStatus("Updating\u{2026}", withIndicator: true)
}
private func batchUpdateCompleted(results: BatchUpdateCompletedNotification) {
if let toolbar = navigationController?.toolbar as? ResourceDownloadStatusToolbar {
toolbar.displayStatus("Updates successfully downloaded!", withIndicator: false, duration: 3.0)
if results.failures.count == 0 {
toolbar.displayStatus("\(results.successes.count) updates successfully downloaded!", withIndicator: false, duration: 3.0)
} else {
toolbar.displayStatus("Updates complete: \(results.successes.count) successful, \(results.failures.count) failed", withIndicator: false, duration: 3.0)
}
}
restoreNavigation()