diff --git a/TablePro/Core/Plugins/PluginManager.swift b/TablePro/Core/Plugins/PluginManager.swift index 3341bb3898..2cf326b058 100644 --- a/TablePro/Core/Plugins/PluginManager.swift +++ b/TablePro/Core/Plugins/PluginManager.swift @@ -122,14 +122,6 @@ final class PluginManager: ObservableObject { self.builtInPluginsURL = builtInPluginsURL self.userPluginsDir = userPluginsDir self.initialLoadClock = initialLoadClock - Self.clearLegacyNeedsRestartKey(in: userDefaults) - } - - nonisolated private static func clearLegacyNeedsRestartKey(in defaults: UserDefaults) { - let legacyKey = "com.TablePro.needsRestart" - if defaults.object(forKey: legacyKey) != nil { - defaults.removeObject(forKey: legacyKey) - } } nonisolated static func defaultUserPluginsDir() -> URL { @@ -154,13 +146,6 @@ final class PluginManager: ObservableObject { return try? JSONDecoder().decode(RegistryMetadata.self, from: data) } - nonisolated static func bundleVersion(at pluginURL: URL) -> String? { - guard let bundle = Bundle(url: pluginURL), - let info = bundle.infoDictionary - else { return nil } - return info["CFBundleShortVersionString"] as? String - } - func saveRegistryMetadata(pluginId: String, pluginURL: URL) { let metadata = RegistryMetadata(pluginId: pluginId) let url = Self.metadataURL(for: pluginURL) @@ -297,6 +282,7 @@ final class PluginManager: ObservableObject { source: source, name: displayName, version: version, + build: Self.bundleBuild(at: url), pluginDescription: pluginDescription, capabilities: capabilities, databaseTypeId: primaryTypeId, @@ -518,6 +504,16 @@ final class PluginManager: ObservableObject { return dictionary["CFBundleShortVersionString"] as? String } + nonisolated static func bundleBuild(at url: URL) -> Int { + let infoPlistURL = url.appendingPathComponent("Contents/Info.plist") + guard let data = try? Data(contentsOf: infoPlistURL), + let plist = try? PropertyListSerialization.propertyList(from: data, format: nil), + let dictionary = plist as? [String: Any] else { + return 0 + } + return (dictionary["CFBundleVersion"] as? String).flatMap { Int($0) } ?? 0 + } + private struct EagerLoadFailure { let url: URL let reason: String @@ -592,6 +588,7 @@ final class PluginManager: ObservableObject { source: source, name: principalClass.pluginName, version: version, + build: Self.bundleBuild(at: url), pluginDescription: principalClass.pluginDescription, capabilities: principalClass.capabilities, databaseTypeId: driverType?.databaseTypeId, @@ -660,11 +657,11 @@ final class PluginManager: ObservableObject { Self.logger.info("Discovered \(self.pendingPluginURLs.count) plugin(s), will load on first use") } - private struct PluginCandidate { + internal struct PluginCandidate { let url: URL let source: PluginSource let bundleId: String - let version: String + let build: Int } nonisolated private static func collectCandidates( @@ -685,13 +682,17 @@ final class PluginManager: ObservableObject { guard let bundle = Bundle(url: itemURL), let bundleId = bundle.bundleIdentifier else { continue } - let version = (bundle.infoDictionary?["CFBundleShortVersionString"] as? String) ?? "0.0.0" - results.append(PluginCandidate(url: itemURL, source: source, bundleId: bundleId, version: version)) + results.append(PluginCandidate( + url: itemURL, + source: source, + bundleId: bundleId, + build: bundleBuild(at: itemURL) + )) } return results } - nonisolated private static func selectWinners( + nonisolated internal static func selectWinners( candidates: [PluginCandidate] ) -> [String: PluginCandidate] { var winners: [String: PluginCandidate] = [:] @@ -700,9 +701,8 @@ final class PluginManager: ObservableObject { winners[candidate.bundleId] = candidate continue } - let order = candidate.version.compare(existing.version, options: .numeric) - let candidateWins = order == .orderedDescending - || (order == .orderedSame && candidate.source == .builtIn) + let candidateWins = candidate.build > existing.build + || (candidate.build == existing.build && candidate.source == .builtIn) if candidateWins { winners[candidate.bundleId] = candidate } @@ -720,10 +720,9 @@ final class PluginManager: ObservableObject { do { try fm.removeItem(at: candidate.url) Self.removeRegistryMetadataFile(for: candidate.url) - let order = candidate.version.compare(winner.version, options: .numeric) - let reason = order == .orderedSame ? "equal version" : "older version" + let reason = candidate.build == winner.build ? "equal build" : "older build" Self.logger.info( - "Pruned user-installed '\(candidate.bundleId)' v\(candidate.version) (\(reason); \(winner.source == .builtIn ? "built-in" : "winning") v\(winner.version) takes precedence)" + "Pruned user-installed '\(candidate.bundleId)' build \(candidate.build) (\(reason); \(winner.source == .builtIn ? "built-in" : "winning") build \(winner.build) takes precedence)" ) } catch { Self.logger.warning( diff --git a/TablePro/Core/Plugins/PluginModels.swift b/TablePro/Core/Plugins/PluginModels.swift index a67501191e..73c76131b2 100644 --- a/TablePro/Core/Plugins/PluginModels.swift +++ b/TablePro/Core/Plugins/PluginModels.swift @@ -1,8 +1,3 @@ -// -// PluginModels.swift -// TablePro -// - import Foundation import TableProPluginKit @@ -13,6 +8,7 @@ struct PluginEntry: Identifiable { let source: PluginSource let name: String var version: String + var build: Int = 0 let pluginDescription: String let capabilities: [PluginCapability] diff --git a/TableProTests/Core/Plugins/PluginBundleVersionTests.swift b/TableProTests/Core/Plugins/PluginBundleVersionTests.swift index 2f9d7bf27c..00957fac6a 100644 --- a/TableProTests/Core/Plugins/PluginBundleVersionTests.swift +++ b/TableProTests/Core/Plugins/PluginBundleVersionTests.swift @@ -1,8 +1,3 @@ -// -// PluginBundleVersionTests.swift -// TableProTests -// - import Foundation @testable import TablePro import Testing @@ -15,21 +10,14 @@ struct PluginBundleVersionTests { return dir } - private func writeBundle(at directory: URL, name: String, version: String) throws -> URL { + private func writeBundle(at directory: URL, name: String, info: [String: String]) throws -> URL { let bundle = directory.appendingPathComponent("\(name).tableplugin", isDirectory: true) let contents = bundle.appendingPathComponent("Contents", isDirectory: true) try FileManager.default.createDirectory(at: contents, withIntermediateDirectories: true) - let payload = """ - - - - - CFBundleIdentifiercom.example.\(name) - CFBundleShortVersionString\(version) - - - """ - try payload.write(to: contents.appendingPathComponent("Info.plist"), atomically: true, encoding: .utf8) + var plist = info + plist["CFBundleIdentifier"] = "com.example.\(name)" + let data = try PropertyListSerialization.data(fromPropertyList: plist, format: .xml, options: 0) + try data.write(to: contents.appendingPathComponent("Info.plist")) return bundle } @@ -38,7 +26,7 @@ struct PluginBundleVersionTests { let dir = try makeTempDir() defer { try? FileManager.default.removeItem(at: dir) } - let bundle = try writeBundle(at: dir, name: "Driver", version: "1.2.3") + let bundle = try writeBundle(at: dir, name: "Driver", info: ["CFBundleShortVersionString": "1.2.3"]) #expect(PluginManager.bundleShortVersion(at: bundle) == "1.2.3") } @@ -47,18 +35,7 @@ struct PluginBundleVersionTests { let dir = try makeTempDir() defer { try? FileManager.default.removeItem(at: dir) } - let bundle = dir.appendingPathComponent("Driver.tableplugin", isDirectory: true) - let contents = bundle.appendingPathComponent("Contents", isDirectory: true) - try FileManager.default.createDirectory(at: contents, withIntermediateDirectories: true) - let payload = """ - - - - CFBundleIdentifiercom.example.Driver - - """ - try payload.write(to: contents.appendingPathComponent("Info.plist"), atomically: true, encoding: .utf8) - + let bundle = try writeBundle(at: dir, name: "Driver", info: [:]) #expect(PluginManager.bundleShortVersion(at: bundle) == nil) } @@ -67,14 +44,62 @@ struct PluginBundleVersionTests { let dir = try makeTempDir() defer { try? FileManager.default.removeItem(at: dir) } - let bundle = try writeBundle(at: dir, name: "Driver", version: "1.0.0") + let bundle = try writeBundle(at: dir, name: "Driver", info: ["CFBundleShortVersionString": "1.0.0"]) let cached = Bundle(url: bundle) #expect(cached?.infoDictionary?["CFBundleShortVersionString"] as? String == "1.0.0") - _ = try writeBundle(at: dir, name: "Driver", version: "2.0.0") + _ = try writeBundle(at: dir, name: "Driver", info: ["CFBundleShortVersionString": "2.0.0"]) #expect(PluginManager.bundleShortVersion(at: bundle) == "2.0.0") #expect(Bundle(url: bundle)?.infoDictionary?["CFBundleShortVersionString"] as? String == "1.0.0") } + + @Test("bundleBuild reads CFBundleVersion from disk") + func readsBuildFromDisk() throws { + let dir = try makeTempDir() + defer { try? FileManager.default.removeItem(at: dir) } + + let bundle = try writeBundle(at: dir, name: "Driver", info: ["CFBundleVersion": "820099"]) + #expect(PluginManager.bundleBuild(at: bundle) == 820_099) + } + + @Test("bundleBuild is 0 when CFBundleVersion is not an integer", arguments: ["1.2", "abc", ""]) + func buildIsZeroWhenNotAnInteger(value: String) throws { + let dir = try makeTempDir() + defer { try? FileManager.default.removeItem(at: dir) } + + let bundle = try writeBundle(at: dir, name: "Driver", info: ["CFBundleVersion": value]) + #expect(PluginManager.bundleBuild(at: bundle) == 0) + } + + @Test("bundleBuild is 0 when CFBundleVersion is missing") + func buildIsZeroWhenMissing() throws { + let dir = try makeTempDir() + defer { try? FileManager.default.removeItem(at: dir) } + + let bundle = try writeBundle(at: dir, name: "Driver", info: [:]) + #expect(PluginManager.bundleBuild(at: bundle) == 0) + } + + @Test("bundleBuild is 0 when there is no bundle on disk") + func buildIsZeroWithoutBundle() throws { + let dir = try makeTempDir() + defer { try? FileManager.default.removeItem(at: dir) } + + #expect(PluginManager.bundleBuild(at: dir.appendingPathComponent("Missing.tableplugin")) == 0) + } + + @Test("bundleBuild sees the new build after the bundle is replaced in place") + func seesNewBuildAfterInPlaceReplace() throws { + let dir = try makeTempDir() + defer { try? FileManager.default.removeItem(at: dir) } + + let bundle = try writeBundle(at: dir, name: "Driver", info: ["CFBundleVersion": "820001"]) + #expect(Bundle(url: bundle)?.infoDictionary?["CFBundleVersion"] as? String == "820001") + + _ = try writeBundle(at: dir, name: "Driver", info: ["CFBundleVersion": "820099"]) + + #expect(PluginManager.bundleBuild(at: bundle) == 820_099) + } } diff --git a/TableProTests/Core/Plugins/PluginWinnerSelectionTests.swift b/TableProTests/Core/Plugins/PluginWinnerSelectionTests.swift new file mode 100644 index 0000000000..ef85a06574 --- /dev/null +++ b/TableProTests/Core/Plugins/PluginWinnerSelectionTests.swift @@ -0,0 +1,61 @@ +import Foundation +@testable import TablePro +import Testing + +struct PluginWinnerSelectionTests { + private let bundleId = "com.TablePro.ExampleDriver" + + private func candidate(_ source: PluginSource, build: Int) -> PluginManager.PluginCandidate { + let folder = source == .builtIn ? "/Applications/TablePro.app/Contents/PlugIns" : "/tmp/Plugins" + return PluginManager.PluginCandidate( + url: URL(fileURLWithPath: "\(folder)/ExampleDriver.tableplugin"), + source: source, + bundleId: bundleId, + build: build + ) + } + + private func winner( + builtIn: PluginManager.PluginCandidate, + user: PluginManager.PluginCandidate, + userFirst: Bool + ) throws -> PluginManager.PluginCandidate { + let candidates = userFirst ? [user, builtIn] : [builtIn, user] + let winners = PluginManager.selectWinners(candidates: candidates) + #expect(winners.count == 1) + return try #require(winners[bundleId]) + } + + @Test("A user copy with a lower build loses to the built-in copy", arguments: [false, true]) + func lowerBuildUserCopyLoses(userFirst: Bool) throws { + let builtIn = candidate(.builtIn, build: 820_099) + let user = candidate(.userInstalled, build: 820_001) + + let picked = try winner(builtIn: builtIn, user: user, userFirst: userFirst) + + #expect(picked.source == .builtIn) + #expect(picked.url == builtIn.url) + } + + @Test("A user copy with a higher build wins over the built-in copy", arguments: [false, true]) + func higherBuildUserCopyWins(userFirst: Bool) throws { + let builtIn = candidate(.builtIn, build: 820_099) + let user = candidate(.userInstalled, build: 820_199) + + let picked = try winner(builtIn: builtIn, user: user, userFirst: userFirst) + + #expect(picked.source == .userInstalled) + #expect(picked.url == user.url) + } + + @Test("An equal build goes to the built-in copy", arguments: [false, true]) + func tieGoesToBuiltIn(userFirst: Bool) throws { + let builtIn = candidate(.builtIn, build: 1) + let user = candidate(.userInstalled, build: 1) + + let picked = try winner(builtIn: builtIn, user: user, userFirst: userFirst) + + #expect(picked.source == .builtIn) + #expect(picked.url == builtIn.url) + } +}