diff --git a/Xcodes/Backend/AppState+Update.swift b/Xcodes/Backend/AppState+Update.swift index cd2952c9..da0ee25f 100644 --- a/Xcodes/Backend/AppState+Update.swift +++ b/Xcodes/Backend/AppState+Update.swift @@ -13,11 +13,14 @@ extension AppState { } func updateIfNeeded() { + // Never cancel a full refresh just because the window became active again. + if isUpdating, updateTaskIsFullRefresh { return } guard - isReadyForUpdate + isReadyForUpdate || !hasRefreshedAvailableXcodesThisLaunch else { updateTask?.cancel() let taskID = UUID() + updateTaskIsFullRefresh = false updateTaskID = taskID let task = Task { @MainActor in defer { @@ -35,8 +38,19 @@ extension AppState { update() as Void } - func update() { - guard !isUpdating else { return } + /// Refreshes installed and available Xcodes and runtimes. + /// - Parameter restartingInFlightUpdate: Cancel a running refresh and start over, e.g. when the + /// data source or sign-in state changed and the running refresh would return stale results. + func update(restartingInFlightUpdate: Bool = false) { + if isUpdating { + // A quick installed-only rescan is always superseded; a full refresh only when asked. + guard restartingInFlightUpdate || !updateTaskIsFullRefresh else { return } + updateTask?.cancel() + updateTask = nil + updateTaskID = nil + } + hasRefreshedAvailableXcodesThisLaunch = true + updateTaskIsFullRefresh = true updateDownloadableRuntimes() updateInstalledRuntimes() @@ -58,6 +72,9 @@ extension AppState { Current.defaults.setDate(Current.date(), forKey: "lastUpdated") } catch is CancellationError { } catch { + // A restarted refresh can surface its cancellation as a URLError instead + guard !Task.isCancelled else { return } + // Prevent setting the app state error if it is an invalid session, we will present the sign in view instead if error as? AuthenticationError != .invalidSession { self.error = error diff --git a/Xcodes/Backend/AppState.swift b/Xcodes/Backend/AppState.swift index 6ac509bb..e9d250f8 100644 --- a/Xcodes/Backend/AppState.swift +++ b/Xcodes/Backend/AppState.swift @@ -45,7 +45,10 @@ class AppState: ObservableObject { @Published var authenticationState: AuthenticationState = .unauthenticated @Published var availableXcodes: [AvailableXcode] = [] { willSet { - if !Self.newlyAvailableXcodes(oldXcodes: availableXcodes, newXcodes: newValue).isEmpty { + if isChangingDataSource { + // The sources identify releases differently, so everything would look new. + isChangingDataSource = false + } else if !Self.newlyAvailableXcodes(oldXcodes: availableXcodes, newXcodes: newValue).isEmpty { Current.notificationManager.scheduleNotification(title: localizeString("Notification.NewXcodeVersion.Title"), body: localizeString("Notification.NewXcodeVersion.Body"), category: .normal) } updateAllXcodes( @@ -83,6 +86,12 @@ class AppState: ObservableObject { @Published var updateTask: Task? var updateTaskID: UUID? var isUpdating: Bool { updateTask != nil } + /// Whether `updateTask` fetches available Xcodes, as opposed to only rescanning installed ones. + var updateTaskIsFullRefresh = false + /// The available Xcode list is always refreshed once per launch, regardless of cache age. + var hasRefreshedAvailableXcodesThisLaunch = false + /// Set while refreshing after a data source change, so the new list isn't announced as new versions + var isChangingDataSource = false @Published var presentedSheet: XcodesSheet? = nil @Published var isProcessingAuthRequest = false private var authenticationRequestID: UUID? @@ -480,8 +489,12 @@ class AppState: ObservableObject { authenticationTaskID = nil } } + let wasAuthenticated = isAuthenticated do { try await operation() + if !wasAuthenticated, isAuthenticated { + refreshAfterSignIn() + } } catch is CancellationError { } catch { // performAuthenticationRequest owns auth error presentation. @@ -489,6 +502,17 @@ class AppState: ObservableObject { } } + private var isAuthenticated: Bool { + if case .authenticated = authenticationState { return true } + return false + } + + /// Signing in unlocks the Apple data source, so restart any refresh that ran without a session. + private func refreshAfterSignIn() { + guard !isTesting else { return } + update(restartingInFlightUpdate: dataSource == .apple) + } + func signOut() { clearLoginCredentials() Current.network.signout() diff --git a/Xcodes/Frontend/Preferences/DownloadPreferencePane.swift b/Xcodes/Frontend/Preferences/DownloadPreferencePane.swift index db082ff2..13143f2f 100644 --- a/Xcodes/Frontend/Preferences/DownloadPreferencePane.swift +++ b/Xcodes/Frontend/Preferences/DownloadPreferencePane.swift @@ -27,6 +27,11 @@ struct DownloadPreferencePane: View { } .groupBoxStyle(PreferencesGroupBoxStyle()) .disabled(dataSource.isManaged) + .onChange(of: dataSource) { + // The cached list belongs to the previous source, so fetch the new one right away. + appState.isChangingDataSource = true + appState.update(restartingInFlightUpdate: true) + } GroupBox(label: Text("Downloader")) { VStack(alignment: .leading) { diff --git a/Xcodes/Frontend/XcodeList/MainToolbar.swift b/Xcodes/Frontend/XcodeList/MainToolbar.swift index f2f645f0..56fddc76 100644 --- a/Xcodes/Frontend/XcodeList/MainToolbar.swift +++ b/Xcodes/Frontend/XcodeList/MainToolbar.swift @@ -16,7 +16,7 @@ struct MainToolbarModifier: ViewModifier { ToolbarItemGroup { ProgressButton( isInProgress: appState.isUpdating, - action: appState.update + action: { appState.update() } ) { Label("Refresh", systemImage: "arrow.clockwise") } diff --git a/XcodesTests/AppStateUpdateTests.swift b/XcodesTests/AppStateUpdateTests.swift index 299e4f01..24fc3753 100644 --- a/XcodesTests/AppStateUpdateTests.swift +++ b/XcodesTests/AppStateUpdateTests.swift @@ -45,6 +45,7 @@ class AppStateUpdateTests: XCTestCase { ) ] Current.defaults.date = { _ in Date.mock() } + subject.hasRefreshedAvailableXcodesThisLaunch = true let continuations = AppStateUpdateTestLockedBox<[CheckedContinuation]>([]) Current.shell.xcodeSelectPrintPath = { @@ -78,6 +79,66 @@ class AppStateUpdateTests: XCTestCase { XCTAssertEqual(subject.selectedXcodePath, "/Applications/Xcode-Beta.app") } + func test_UpdateIfNeeded_FirstCallThisLaunch_RefreshesEvenWithFreshCache() async throws { + subject.availableXcodes = [ + AvailableXcode(version: Version("0.0.0")!, url: URL(string: "https://apple.com/xcode.xip")!, filename: "mock.xip", releaseDate: nil) + ] + Current.defaults.date = { _ in Date.mock() } + XCTAssertFalse(subject.isReadyForUpdate) + let continuations = blockXcodeSelect() + + subject.updateIfNeeded() + XCTAssertTrue(subject.updateTaskIsFullRefresh) + XCTAssertTrue(subject.hasRefreshedAvailableXcodesThisLaunch) + let fullRefreshTaskID = try XCTUnwrap(subject.updateTaskID) + + // Becoming active again must not cancel the running full refresh + subject.updateIfNeeded() + XCTAssertEqual(subject.updateTaskID, fullRefreshTaskID) + XCTAssertTrue(subject.updateTaskIsFullRefresh) + + await cancelUpdateTask(releasing: continuations) + } + + func test_Update_OnlyRestartsRunningFullRefreshWhenAsked() async throws { + let continuations = blockXcodeSelect() + + subject.update() + let firstTaskID = try XCTUnwrap(subject.updateTaskID) + + subject.update() + XCTAssertEqual(subject.updateTaskID, firstTaskID) + + subject.update(restartingInFlightUpdate: true) + XCTAssertNotNil(subject.updateTaskID) + XCTAssertNotEqual(subject.updateTaskID, firstTaskID) + + await cancelUpdateTask(releasing: continuations) + } + + private func blockXcodeSelect() -> AppStateUpdateTestLockedBox<[CheckedContinuation]> { + let continuations = AppStateUpdateTestLockedBox<[CheckedContinuation]>([]) + Current.shell.xcodeSelectPrintPath = { + try await withCheckedThrowingContinuation { continuation in + continuations.withValue { $0.append(continuation) } + } + } + return continuations + } + + private func cancelUpdateTask(releasing continuations: AppStateUpdateTestLockedBox<[CheckedContinuation]>) async { + let task = subject.updateTask + task?.cancel() + for _ in 0..<100 where continuations.read({ $0.isEmpty }) { + await Task.yield() + } + continuations.withValue { pending in + pending.forEach { $0.resume(throwing: CancellationError()) } + pending.removeAll() + } + await task?.value + } + func testDoesNotReplaceInstallState() throws { subject.allXcodes = [ Xcode(version: Version("0.0.0")!, installState: .installing(.unarchiving), selected: false, icon: nil)