feat(mac): handled drag and drop errors

display alert when drag and drop fails
handle installation errors
clean up un-installable .kmp files

mac/Config/Config/ConfigDebugView.swift

@@ -12,6 +12,9 @@ import KeymanSettings
 struct ConfigDebugView: View {
   @EnvironmentObject var settings: SettingsContainer
   @State private var isShowingSheet = false
+  @State private var dropError: DropKmpError?
+  @State private var isShowingDropKmpAlert = false
+  @State private var alertMessage = ""
   @State private var isHovering = false
 
   var body: some View {
@@ -43,7 +46,7 @@ struct ConfigDebugView: View {
       }
 
       VStack {
-        Text(settings.dragStatusMessage)
+        Text(settings.dropStatusMessage)
           .font(.system(.body, design: .monospaced))
           .multilineTextAlignment(.center)
           .padding()
@@ -56,16 +59,33 @@ struct ConfigDebugView: View {
           )
         // Accept URL drops
           .dropDestination(for: URL.self) { urls, _ in
-            guard let archiveURL = urls.first, urls.count == 1 else {
-              settings.dragStatusMessage = "Drop exactly one file."
-              return false
+            // reject drop if it is more than one file
+            guard let droppedFileUrl = urls.first, urls.count == 1 else {
+              let error = DropKmpError.tooManyFiles
+              self.alertMessage = error.localizedDescription
+              self.isShowingDropKmpAlert = true
+              return false // the drop failed
+            }
+            do {
+              try settings.processDroppedKmpFile(at: droppedFileUrl)
+              return true // the drop was successful
+            } catch {
+              self.alertMessage = error.localizedDescription
+              self.isShowingDropKmpAlert = true
+              return false
+              
             }
-            return settings.processDraggedKmpFile(from: archiveURL)
           } isTargeted: { hovering in
             isHovering = hovering
           }
       }
       .padding()
+      // alert riggers automatically when $dropError becomes non-nil
+      .alert("Package Installation Failed", isPresented: $isShowingDropKmpAlert) {
+          Button("OK", role: .cancel) { }
+      } message: {
+          Text(alertMessage)
+      }
 
       ScrollView {
         VStack(alignment: .leading, spacing: 6) {

mac/KeymanSettings/Sources/KeymanSettings/PackageRepo.swift

@@ -19,6 +19,6 @@ public protocol PackageRepo {
   func loadSinglePackage(packageUrl: URL) throws -> KeymanPackage
   func getDownloadUrl(for kmpFilename: String) -> URL
   func getUnzipDestinationUrl(for packageName: String) -> URL
-  func getInstallationUrlForPackageName(packageName: String) -> URL
+  func buildInstallationUrlForPackageName(packageName: String) -> URL
   func cleanupTempDirectory()
 }

mac/KeymanSettings/Sources/KeymanSettings/SettingsContainer.swift

@@ -40,10 +40,26 @@ public extension Notification.Name {
   static let packageDowngradeRequested = Notification.Name("com.keyman.package.downgrade.requested")
 }
 
-public enum SettingsError: Error {
-  case unknownPackage
+// define LocalizedError so that UI can present a localizable message
+// when the attempt to install a KMP file using drag and drop fails
+public enum DropKmpError: LocalizedError {
+  case invalidFileType(String)
+  case alreadyInstalled(String)
+  case installFailed(String)
+  case tooManyFiles
+  
+  public var errorDescription: String? {
+    switch self {
+    case .invalidFileType(let fileName): return "The file \(fileName) is not a .KMP file."
+    case .alreadyInstalled(let fileName): return "The package \(fileName) is already installed."
+    case .installFailed(let fileName): return "The file \(fileName) could not be installed."
+    case .tooManyFiles: return "Only a single .KMP file can be installed at a time."
+    }
+  }
 }
 
+private let kmpFileExtension = ".kmp"
+
 @MainActor // run on the main actor since data is published directly to the UI
 public class SettingsContainer : ObservableObject {
   // installed packages are loaded from disk, each package may contain one or more keyboard
@@ -61,7 +77,7 @@ public class SettingsContainer : ObservableObject {
   // (Consider installedPackages as the source of truth and these arrays for presentation purposes.)
   @Published public private(set) var singleKeyboardPackages: [KeymanPackage]
   @Published public private(set) var multiKeyboardPackages: [KeymanPackage]
-  @Published public var dragStatusMessage = "Drag a single .kmp archive here"
+  @Published public var dropStatusMessage = "Drag a single .kmp archive here"
 
   // when a new package is downloaded, it is tracked here
   public private(set) var packageDownload: PackageDownload? = nil
@@ -230,52 +246,6 @@ public class SettingsContainer : ObservableObject {
    return false
   }
 
-  public func processDraggedKmpFile(from fileLocation: URL) -> Bool {
-    // if the file does not end with .kmp, reject it
-    guard fileLocation.pathExtension.lowercased() == "kmp" else {
-      dragStatusMessage = "Rejected: file must have a .kmp extension."
-      return false
-    }
-    
-    // if we cannot get a URL to the install location, then reject it (should never happen)
-    guard let destinationURL = getInstalledPackageUrl(for: fileLocation) else {
-      dragStatusMessage = "Unable to find application data directory."
-        return false
-    }
-    
-    // if a package of the same name is installed, reject it
-    guard !FileManager.default.fileExists(atPath: destinationURL.path) else {
-      dragStatusMessage = "The package \(destinationURL.lastPathComponent) is already installed."
-        return false
-    }
-    
-    do {
-      try self.installDraggedPackage(from: fileLocation, to: destinationURL)
-      dragStatusMessage = "The package \(destinationURL.lastPathComponent) was installed successfully."
-      return true
-    } catch {
-      dragStatusMessage = "The package \(destinationURL.lastPathComponent) failed to install."
-      return false
-    }
-  }
-
-  func getInstalledPackageUrl(for draggedKmpFile: URL) -> URL? {
-    // package name is filename minus .kmp extension
-    let packageName = draggedKmpFile.lastPathComponent.replacingOccurrences(of: ".kmp", with: "")
-    return self.packageRepository.getInstallationUrlForPackageName(packageName: packageName)
-  }
-
-  func installDraggedPackage(from draggedFileUrl: URL, to installPackageLocation: URL) throws {
-    try self.packageRepository.unzipKmpFile(at: draggedFileUrl, to: installPackageLocation)
-    
-    // load the unzipped package and get a reference to it
-    let newPackage = try self.packageRepository.loadSinglePackage(packageUrl: installPackageLocation)
-    
-    // add the new package to the array and enable its keyboards
-    self.installedPackages.append(newPackage)
-    self.addEnabledKeyboards(for: newPackage)
-  }
-  
   /**
    * Called by the WebView Coordinator before initiating a package download.
    * Creates a PackageDownload instance to manage the state of the package being downloaded with the specified name.
@@ -283,7 +253,7 @@ public class SettingsContainer : ObservableObject {
    */
   public func preparePackageDownload(kmpFileName: String) -> URL? {
     // package name is filename minus .kmp extension
-    let packageName = kmpFileName.replacingOccurrences(of: ".kmp", with: "")
+    let packageName = kmpFileName.replacingOccurrences(of: kmpFileExtension, with: "")
     
     let packageDownload = PackageDownload(filename: kmpFileName, packageName: packageName, packageRepo: self.packageRepository, installedPackages: self.installedPackages)
 
@@ -517,4 +487,86 @@ public class SettingsContainer : ObservableObject {
       }
     }
   }
+  
+  // MARK: Drag and drop Package Installation
+  
+  /**
+   * Attempt to install a package from a KMP file. Called when file is dropped on the Configuration view
+   */
+  public func processDroppedKmpFile(at fileLocation: URL) throws {
+    // get the location where the package will be installed
+    let destinationURL = try self.validateDropUrlForInstallation(from: fileLocation)
+    
+    // install it
+    try self.installDroppedKmpFile(from: fileLocation, to: destinationURL)
+  }
+
+  /**
+   * Validate the URL for the file we are dropping and return the installation location
+   * Throws errors if the URL does not end with .kmp or the same package is already installed
+   */
+  func validateDropUrlForInstallation(from fileLocation: URL) throws -> URL {
+    // if the file does not end with .kmp, reject it
+    guard fileLocation.pathExtension.lowercased() == "kmp" else {
+      dropStatusMessage = "Rejected: file must have a .kmp extension."
+      throw DropKmpError.invalidFileType(fileLocation.lastPathComponent)
+    }
+    
+    // if we cannot get a URL to the install location, then reject it (should never happen)
+    guard let destinationURL = buildInstalledPackageUrl(for: fileLocation) else {
+      dropStatusMessage = "Unable to find application data directory."
+      throw DropKmpError.installFailed(fileLocation.lastPathComponent)
+    }
+    
+    // if a package of the same name is installed, reject it
+    guard !FileManager.default.fileExists(atPath: destinationURL.path) else {
+      dropStatusMessage = "The package \(destinationURL.lastPathComponent) is already installed."
+      throw DropKmpError.alreadyInstalled(fileLocation.lastPathComponent)
+    }
+    
+    return destinationURL
+  }
+  
+  /**
+   * Build the URL where the package will be installed
+   */
+  func buildInstalledPackageUrl(for draggedKmpFile: URL) -> URL? {
+    // package name is filename minus .kmp extension
+    let packageName = draggedKmpFile.lastPathComponent.replacingOccurrences(of: kmpFileExtension, with: "")
+    return self.packageRepository.buildInstallationUrlForPackageName(packageName: packageName)
+  }
+
+  /**
+   * Install the package from the dropped kmp file
+   */
+  func installDroppedKmpFile(from droppedFileUrl: URL, to installPackageLocation: URL) throws {
+    var newPackage: KeymanPackage? = nil
+    
+    try self.packageRepository.unzipKmpFile(at: droppedFileUrl, to: installPackageLocation)
+    
+    do {
+      // load the unzipped package and get a reference to it
+      newPackage = try self.packageRepository.loadSinglePackage(packageUrl: installPackageLocation)
+    }
+    catch {
+      // the package could not be loaded, so delete it from disk
+      do {
+        if FileManager.default.fileExists(atPath: installPackageLocation.path) {
+          try FileManager.default.removeItem(at: installPackageLocation)
+          print("removed uninstalled dropped kmp file at: \(installPackageLocation)")
+        }
+      } catch {
+        print("could not remove uninstalled dropped kmp file: \(error.localizedDescription)")
+      }
+      
+      // re-throw error to notify user of reason installation failed
+      throw error
+    }
+    
+    if let installedPackage = newPackage {
+      // add the newly installed package to the array and enable its keyboards
+      self.installedPackages.append(installedPackage)
+      self.addEnabledKeyboards(for: installedPackage)
+    }
+  }
 }

mac/KeymanSettings/Sources/Persistence/PackageDownload.swift

@@ -26,7 +26,7 @@ public class PackageDownload {
     self.packageRepository = packageRepo
     self.temporaryKmpFileLocation = self.packageRepository.getDownloadUrl(for: filename)
     self.temporaryPackageLocation = self.packageRepository.getUnzipDestinationUrl(for: packageName)
-    self.installPackageLocation = self.packageRepository.getInstallationUrlForPackageName(packageName: packageName)
+    self.installPackageLocation = self.packageRepository.buildInstallationUrlForPackageName(packageName: packageName)
     self.installedPackages = installedPackages
     
     // cannot be initialized until after download when packageName of new package is known

mac/KeymanSettings/Sources/Persistence/PackageRepository.swift

@@ -10,7 +10,7 @@
 
 import Foundation
 
-enum LoadPackageError: Error {
+public enum LoadPackageError: LocalizedError {
   case containsNoFiles
   case containsNoKeyboards
   case kmpJsonFileUnreadable
@@ -19,33 +19,29 @@ enum LoadPackageError: Error {
   case missingKeyboardId
   case missingKeyboardVersion
   case missingKmxFile
+  
+  public var errorDescription: String? {
+    switch self {
+    case .containsNoFiles: return "The keyboard package contains no files."
+    case .containsNoKeyboards: return "The keyboard package contains no keyboards."
+    case .kmpJsonFileUnreadable: return "The package's kmp.json file could not be parsed."
+    case .kmpJsonFileNotFound: return "The package's kmp.json file was not found."
+    case .missingKeyboardName: return "A keyboard in the package has no name."
+    case .missingKeyboardId: return "A keyboard in the package has no ID."
+    case .missingKeyboardVersion: return "A keyboard in the package has no version."
+    case .missingKmxFile: return "A keyboard in the package has no corresponding KMX file."
+    }
+  }
 }
 
-enum InstallPackageError: Error {
+enum InstallPackageError: LocalizedError {
   case invalidUrl
   case unzipError
-}
-
-// Conform to LocalizedError to provide the description
-extension LoadPackageError: LocalizedError {
-  var errorDescription: String? {
+  
+  public var errorDescription: String? {
     switch self {
-    case .containsNoFiles:
-      return NSLocalizedString("The package contains no files.", comment: "")
-    case .containsNoKeyboards:
-      return NSLocalizedString("The package contains no keyboards", comment: "")
-    case .kmpJsonFileUnreadable:
-      return NSLocalizedString("The package's kmp.json file could not be parsed", comment: "")
-    case .kmpJsonFileNotFound:
-      return NSLocalizedString("The package's kmp.json file was not found", comment: "")
-    case .missingKeyboardName:
-      return NSLocalizedString("A keyboard in the package has no name", comment: "")
-    case .missingKeyboardId:
-      return NSLocalizedString("A keyboard in the package has no id", comment: "")
-    case .missingKeyboardVersion:
-      return NSLocalizedString("A keyboard in the package has no version", comment: "")
-    case .missingKmxFile:
-      return NSLocalizedString("A keyboard in the package has no corresponding KMX file", comment: "")
+    case .invalidUrl: return "The URL is not valid."
+    case .unzipError: return "The keyboard package could not be unzipped."
     }
   }
 }
@@ -171,9 +167,9 @@ public class PackageRepository: PackageRepo {
     return self.pathUtil.keyman19TempDirectory.appendingPathComponent(packageName)
   }
   /**
-   * get the url to where the specified package should be installed
+   * build the URL where the specified package will be installed
    */
-  public func getInstallationUrlForPackageName(packageName: String) -> URL {
+  public func buildInstallationUrlForPackageName(packageName: String) -> URL {
     return self.pathUtil.keyman19PackagesDirectory.appendingPathComponent(packageName)
   }
 

mac/KeymanSettings/Tests/KeymanSettingsTests/RepoStubs.swift

@@ -94,7 +94,7 @@ class PackageRepoStub: PackageRepo {
     return URL(fileURLWithPath: "")
   }
   
-  func getInstallationUrlForPackageName(packageName: String) -> URL {
+  func buildInstallationUrlForPackageName(packageName: String) -> URL {
     return URL(fileURLWithPath: "")
   }