From 9ebb01805d735ef83bece9edb3acb405a6888651 Mon Sep 17 00:00:00 2001 From: Ross Goldberg <484615+rgoldberg@users.noreply.github.com> Date: Mon, 28 Oct 2024 11:26:11 -0400 Subject: [PATCH] Replace clunky `ExternalCommand` code that starts new processes with Apple library calls. Resolve #620 Signed-off-by: Ross Goldberg <484615+rgoldberg@users.noreply.github.com> --- Sources/mas/Commands/Home.swift | 31 ++++----- Sources/mas/Commands/Open.swift | 45 +++++-------- Sources/mas/Commands/Vendor.swift | 39 +++++------- .../ITunesSearchAppStoreSearcher.swift | 6 +- .../ExternalCommands/ExternalCommand.swift | 63 ------------------- .../ExternalCommands/OpenSystemCommand.swift | 23 ------- .../SysCtlSystemCommand.swift | 41 ------------ Sources/mas/Network/URL.swift | 32 ++++++++++ Tests/masTests/Commands/HomeSpec.swift | 10 +-- Tests/masTests/Commands/VendorSpec.swift | 10 +-- .../MockOpenSystemCommand.swift | 27 -------- .../OpenSystemCommandSpec.swift | 30 --------- 12 files changed, 83 insertions(+), 274 deletions(-) delete mode 100644 Sources/mas/ExternalCommands/ExternalCommand.swift delete mode 100644 Sources/mas/ExternalCommands/OpenSystemCommand.swift delete mode 100644 Sources/mas/ExternalCommands/SysCtlSystemCommand.swift create mode 100644 Sources/mas/Network/URL.swift delete mode 100644 Tests/masTests/ExternalCommands/MockOpenSystemCommand.swift delete mode 100644 Tests/masTests/ExternalCommands/OpenSystemCommandSpec.swift diff --git a/Sources/mas/Commands/Home.swift b/Sources/mas/Commands/Home.swift index 8d86a5f..531b6c5 100644 --- a/Sources/mas/Commands/Home.swift +++ b/Sources/mas/Commands/Home.swift @@ -7,6 +7,7 @@ // import ArgumentParser +import Foundation extension MAS { /// Opens app page on MAS Preview. Uses the iTunes Lookup API: @@ -21,29 +22,19 @@ extension MAS { /// Runs the command. func run() throws { - try run(searcher: ITunesSearchAppStoreSearcher(), openCommand: OpenSystemCommand()) + try run(searcher: ITunesSearchAppStoreSearcher()) } - func run(searcher: AppStoreSearcher, openCommand: ExternalCommand) throws { - do { - guard let result = try searcher.lookup(appID: appID).wait() else { - throw MASError.noSearchResultsFound - } - - do { - try openCommand.run(arguments: result.trackViewUrl) - } catch { - printError("Unable to launch open command") - throw MASError.searchFailed - } - if openCommand.failed { - let reason = openCommand.process.terminationReason - printError("Open failed: (\(reason)) \(openCommand.stderr)") - throw MASError.searchFailed - } - } catch { - throw error as? MASError ?? .searchFailed + func run(searcher: AppStoreSearcher) throws { + guard let result = try searcher.lookup(appID: appID).wait() else { + throw MASError.noSearchResultsFound } + + guard let url = URL(string: result.trackViewUrl) else { + throw MASError.runtimeError("Unable to construct URL from: \(result.trackViewUrl)") + } + + try url.open().wait() } } } diff --git a/Sources/mas/Commands/Open.swift b/Sources/mas/Commands/Open.swift index 09f0c8d..b56f00b 100644 --- a/Sources/mas/Commands/Open.swift +++ b/Sources/mas/Commands/Open.swift @@ -8,7 +8,6 @@ import AppKit import ArgumentParser -import Foundation import PromiseKit private let masScheme = "macappstore" @@ -35,7 +34,7 @@ extension MAS { try openMacAppStore().wait() return } - try openInMacAppStore(pageForAppID: appID, searcher: searcher).wait() + try openInMacAppStore(pageForAppID: appID, searcher: searcher) } } } @@ -63,32 +62,20 @@ private func openMacAppStore() -> Promise { } } -private func openInMacAppStore(pageForAppID appID: AppID, searcher: AppStoreSearcher) -> Promise { - Promise { seal in - guard let result = try searcher.lookup(appID: appID).wait() else { - throw MASError.runtimeError("Unknown app ID \(appID)") - } - - guard var urlComponents = URLComponents(string: result.trackViewUrl) else { - throw MASError.runtimeError("Unable to construct URL from: \(result.trackViewUrl)") - } - - urlComponents.scheme = masScheme - - guard let url = urlComponents.url else { - throw MASError.runtimeError("Unable to construct URL from: \(urlComponents)") - } - - if #available(macOS 10.15, *) { - NSWorkspace.shared.open(url, configuration: NSWorkspace.OpenConfiguration()) { _, error in - if let error { - seal.reject(error) - } - seal.fulfill(()) - } - } else { - NSWorkspace.shared.open(url) - seal.fulfill(()) - } +private func openInMacAppStore(pageForAppID appID: AppID, searcher: AppStoreSearcher) throws { + guard let result = try searcher.lookup(appID: appID).wait() else { + throw MASError.runtimeError("Unknown app ID \(appID)") } + + guard var urlComponents = URLComponents(string: result.trackViewUrl) else { + throw MASError.runtimeError("Unable to construct URL from: \(result.trackViewUrl)") + } + + urlComponents.scheme = masScheme + + guard let url = urlComponents.url else { + throw MASError.runtimeError("Unable to construct URL from: \(urlComponents)") + } + + try url.open().wait() } diff --git a/Sources/mas/Commands/Vendor.swift b/Sources/mas/Commands/Vendor.swift index d303c94..7256d85 100644 --- a/Sources/mas/Commands/Vendor.swift +++ b/Sources/mas/Commands/Vendor.swift @@ -7,6 +7,7 @@ // import ArgumentParser +import Foundation extension MAS { /// Opens vendor's app page in a browser. Uses the iTunes Lookup API: @@ -21,33 +22,23 @@ extension MAS { /// Runs the command. func run() throws { - try run(searcher: ITunesSearchAppStoreSearcher(), openCommand: OpenSystemCommand()) + try run(searcher: ITunesSearchAppStoreSearcher()) } - func run(searcher: AppStoreSearcher, openCommand: ExternalCommand) throws { - do { - guard let result = try searcher.lookup(appID: appID).wait() else { - throw MASError.noSearchResultsFound - } - - guard let vendorWebsite = result.sellerUrl else { - throw MASError.noVendorWebsite - } - - do { - try openCommand.run(arguments: vendorWebsite) - } catch { - printError("Unable to launch open command") - throw MASError.searchFailed - } - if openCommand.failed { - let reason = openCommand.process.terminationReason - printError("Open failed: (\(reason)) \(openCommand.stderr)") - throw MASError.searchFailed - } - } catch { - throw error as? MASError ?? .searchFailed + func run(searcher: AppStoreSearcher) throws { + guard let result = try searcher.lookup(appID: appID).wait() else { + throw MASError.noSearchResultsFound } + + guard let urlString = result.sellerUrl else { + throw MASError.noVendorWebsite + } + + guard let url = URL(string: urlString) else { + throw MASError.runtimeError("Unable to construct URL from: \(urlString)") + } + + try url.open().wait() } } } diff --git a/Sources/mas/Controllers/ITunesSearchAppStoreSearcher.swift b/Sources/mas/Controllers/ITunesSearchAppStoreSearcher.swift index cc1939f..babaab9 100644 --- a/Sources/mas/Controllers/ITunesSearchAppStoreSearcher.swift +++ b/Sources/mas/Controllers/ITunesSearchAppStoreSearcher.swift @@ -85,9 +85,9 @@ class ITunesSearchAppStoreSearcher: AppStoreSearcher { // Search for apps for compatible platforms, in order of preference. // Macs with Apple Silicon can run iPad and iPhone apps. var entities = [Entity.desktopSoftware] - if SysCtlSystemCommand.isAppleSilicon { - entities += [.iPadSoftware, .iPhoneSoftware] - } + #if arch(arm64) + entities += [.iPadSoftware, .iPhoneSoftware] + #endif let results = entities.map { entity in guard let url = searchURL(for: searchTerm, inCountry: country, ofEntity: entity) else { diff --git a/Sources/mas/ExternalCommands/ExternalCommand.swift b/Sources/mas/ExternalCommands/ExternalCommand.swift deleted file mode 100644 index b5a3487..0000000 --- a/Sources/mas/ExternalCommands/ExternalCommand.swift +++ /dev/null @@ -1,63 +0,0 @@ -// -// ExternalCommand.swift -// mas -// -// Created by Ben Chatelain on 1/1/19. -// Copyright © 2019 mas-cli. All rights reserved. -// - -import Foundation - -/// Represents a CLI command. -protocol ExternalCommand { - var binaryPath: String { get set } - - var process: Process { get } - - var stdout: String { get } - var stderr: String { get } - var stdoutPipe: Pipe { get } - var stderrPipe: Pipe { get } - - var exitCode: Int32 { get } - var succeeded: Bool { get } - var failed: Bool { get } - - /// Runs the command. - func run(arguments: String...) throws -} - -/// Common implementation -extension ExternalCommand { - var stdout: String { - let data = stdoutPipe.fileHandleForReading.readDataToEndOfFile() - return String(data: data, encoding: .utf8) ?? "" - } - - var stderr: String { - let data = stderrPipe.fileHandleForReading.readDataToEndOfFile() - return String(data: data, encoding: .utf8) ?? "" - } - - var exitCode: Int32 { - process.terminationStatus - } - - var succeeded: Bool { - process.terminationReason == .exit && exitCode == 0 - } - - var failed: Bool { - !succeeded - } - - /// Runs the command. - func run(arguments: String...) throws { - process.standardOutput = stdoutPipe - process.standardError = stderrPipe - process.arguments = arguments - process.executableURL = URL(fileURLWithPath: binaryPath) - try process.run() - process.waitUntilExit() - } -} diff --git a/Sources/mas/ExternalCommands/OpenSystemCommand.swift b/Sources/mas/ExternalCommands/OpenSystemCommand.swift deleted file mode 100644 index 5d5e3cc..0000000 --- a/Sources/mas/ExternalCommands/OpenSystemCommand.swift +++ /dev/null @@ -1,23 +0,0 @@ -// -// OpenSystemCommand.swift -// mas -// -// Created by Ben Chatelain on 1/2/19. -// Copyright © 2019 mas-cli. All rights reserved. -// - -import Foundation - -/// Wrapper for the external 'open' system command (https://ss64.com/osx/open.html). -struct OpenSystemCommand: ExternalCommand { - var binaryPath: String - - let process = Process() - - let stdoutPipe = Pipe() - let stderrPipe = Pipe() - - init(binaryPath: String = "/usr/bin/open") { - self.binaryPath = binaryPath - } -} diff --git a/Sources/mas/ExternalCommands/SysCtlSystemCommand.swift b/Sources/mas/ExternalCommands/SysCtlSystemCommand.swift deleted file mode 100644 index 1d04a2e..0000000 --- a/Sources/mas/ExternalCommands/SysCtlSystemCommand.swift +++ /dev/null @@ -1,41 +0,0 @@ -// -// SysCtlSystemCommand.swift -// mas -// -// Created by Chris Araman on 6/3/21. -// Copyright © 2021 mas-cli. All rights reserved. -// - -import Foundation - -/// Wrapper for the external 'sysctl' system command. -/// -/// See - https://ss64.com/osx/sysctl.html -struct SysCtlSystemCommand: ExternalCommand { - static var isAppleSilicon: Bool = { - let sysctl = Self() - do { - // Returns 1 on Apple Silicon even when run in an Intel context in Rosetta. - try sysctl.run(arguments: "-in", "hw.optional.arm64") - } catch { - fatalError("sysctl failed") - } - - guard sysctl.succeeded else { - fatalError("sysctl failed") - } - - return sysctl.stdout.trimmingCharacters(in: .newlines) == "1" - }() - - let process = Process() - - let stdoutPipe = Pipe() - let stderrPipe = Pipe() - - var binaryPath: String - - init(binaryPath: String = "/usr/sbin/sysctl") { - self.binaryPath = binaryPath - } -} diff --git a/Sources/mas/Network/URL.swift b/Sources/mas/Network/URL.swift new file mode 100644 index 0000000..538c1a9 --- /dev/null +++ b/Sources/mas/Network/URL.swift @@ -0,0 +1,32 @@ +// +// URL.swift +// mas +// +// Created by Ross Goldberg on 2024-10-28. +// Copyright © 2024 mas-cli. All rights reserved. +// + +import AppKit +import Foundation +import PromiseKit + +extension URL { + func open() -> Promise { + Promise { seal in + if #available(macOS 10.15, *) { + NSWorkspace.shared.open(self, configuration: NSWorkspace.OpenConfiguration()) { _, error in + if let error { + seal.reject(error) + } + seal.fulfill(()) + } + } else { + if NSWorkspace.shared.open(self) { + seal.fulfill(()) + } else { + seal.reject(MASError.runtimeError("Failed to open \(self)")) + } + } + } + } +} diff --git a/Tests/masTests/Commands/HomeSpec.swift b/Tests/masTests/Commands/HomeSpec.swift index 8d3faac..c92b58b 100644 --- a/Tests/masTests/Commands/HomeSpec.swift +++ b/Tests/masTests/Commands/HomeSpec.swift @@ -14,7 +14,6 @@ import Quick public class HomeSpec: QuickSpec { override public func spec() { let searcher = MockAppStoreSearcher() - let openCommand = MockOpenSystemCommand() beforeSuite { MAS.initialize() @@ -25,13 +24,13 @@ public class HomeSpec: QuickSpec { } it("fails to open app with invalid ID") { expect { - try MAS.Home.parse(["--", "-999"]).run(searcher: searcher, openCommand: openCommand) + try MAS.Home.parse(["--", "-999"]).run(searcher: searcher) } .to(throwError()) } it("can't find app with unknown ID") { expect { - try MAS.Home.parse(["999"]).run(searcher: searcher, openCommand: openCommand) + try MAS.Home.parse(["999"]).run(searcher: searcher) } .to(throwError(MASError.noSearchResultsFound)) } @@ -43,11 +42,8 @@ public class HomeSpec: QuickSpec { ) searcher.apps[mockResult.trackId] = mockResult expect { - try MAS.Home.parse([String(mockResult.trackId)]) - .run(searcher: searcher, openCommand: openCommand) - return openCommand.arguments + try MAS.Home.parse([String(mockResult.trackId)]).run(searcher: searcher) } - == [mockResult.trackViewUrl] } } } diff --git a/Tests/masTests/Commands/VendorSpec.swift b/Tests/masTests/Commands/VendorSpec.swift index 1834cb3..5c7eb95 100644 --- a/Tests/masTests/Commands/VendorSpec.swift +++ b/Tests/masTests/Commands/VendorSpec.swift @@ -14,7 +14,6 @@ import Quick public class VendorSpec: QuickSpec { override public func spec() { let searcher = MockAppStoreSearcher() - let openCommand = MockOpenSystemCommand() beforeSuite { MAS.initialize() @@ -25,13 +24,13 @@ public class VendorSpec: QuickSpec { } it("fails to open app with invalid ID") { expect { - try MAS.Vendor.parse(["--", "-999"]).run(searcher: searcher, openCommand: openCommand) + try MAS.Vendor.parse(["--", "-999"]).run(searcher: searcher) } .to(throwError()) } it("can't find app with unknown ID") { expect { - try MAS.Vendor.parse(["999"]).run(searcher: searcher, openCommand: openCommand) + try MAS.Vendor.parse(["999"]).run(searcher: searcher) } .to(throwError(MASError.noSearchResultsFound)) } @@ -44,11 +43,8 @@ public class VendorSpec: QuickSpec { ) searcher.apps[mockResult.trackId] = mockResult expect { - try MAS.Vendor.parse([String(mockResult.trackId)]) - .run(searcher: searcher, openCommand: openCommand) - return openCommand.arguments + try MAS.Vendor.parse([String(mockResult.trackId)]).run(searcher: searcher) } - == [mockResult.sellerUrl] } } } diff --git a/Tests/masTests/ExternalCommands/MockOpenSystemCommand.swift b/Tests/masTests/ExternalCommands/MockOpenSystemCommand.swift deleted file mode 100644 index da30e13..0000000 --- a/Tests/masTests/ExternalCommands/MockOpenSystemCommand.swift +++ /dev/null @@ -1,27 +0,0 @@ -// -// MockOpenSystemCommand.swift -// masTests -// -// Created by Ben Chatelain on 1/4/19. -// Copyright © 2019 mas-cli. All rights reserved. -// - -import Foundation - -@testable import mas - -class MockOpenSystemCommand: ExternalCommand { - // Stub out protocol logic - var succeeded = true - var arguments: [String] = [] - - // unused - var binaryPath = "/dev/null" - var process = Process() - var stdoutPipe = Pipe() - var stderrPipe = Pipe() - - func run(arguments: String...) throws { - self.arguments = arguments - } -} diff --git a/Tests/masTests/ExternalCommands/OpenSystemCommandSpec.swift b/Tests/masTests/ExternalCommands/OpenSystemCommandSpec.swift deleted file mode 100644 index 9b8c1f5..0000000 --- a/Tests/masTests/ExternalCommands/OpenSystemCommandSpec.swift +++ /dev/null @@ -1,30 +0,0 @@ -// -// OpenSystemCommandSpec.swift -// masTests -// -// Created by Ben Chatelain on 2/24/20. -// Copyright © 2020 mas-cli. All rights reserved. -// - -import Nimble -import Quick - -@testable import mas - -public class OpenSystemCommandSpec: QuickSpec { - override public func spec() { - beforeSuite { - MAS.initialize() - } - describe("open system command") { - context("binary path") { - it("defaults to the macOS open command") { - expect(OpenSystemCommand().binaryPath) == "/usr/bin/open" - } - it("can be overridden") { - expect(OpenSystemCommand(binaryPath: "/dev/null").binaryPath) == "/dev/null" - } - } - } - } -}