From 786ca2de559653437e4e257249049fcbb3eb7389 Mon Sep 17 00:00:00 2001
From: Anh Do <18567+quanganhdo@users.noreply.github.com>
Date: Thu, 5 Dec 2024 14:38:30 -0500
Subject: [PATCH] Fix phased rollout handling (#3625)
Task/Issue URL:
https://app.asana.com/0/1201037661562251/1208867792197198/f
Tech Design URL:
CC:
**Description**:
Fixes automatic update skipping phased rollout group setting.
**Optional E2E tests**:
- [ ] Run PIR E2E tests
Check this to run the Personal Information Removal end to end tests. If
updating CCF, or any PIR related code, tick this.
**Steps to test this PR**:
1. https://app.asana.com/0/0/1208882660080993/f
**Definition of Done**:
* [ ] Does this PR satisfy our [Definition of
Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)?
---
###### Internal references:
[Pull Request Review
Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f)
[Software Engineering
Expectations](https://app.asana.com/0/59792373528535/199064865822552)
[Technical Design
Template](https://app.asana.com/0/59792373528535/184709971311943)
[Pull Request
Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f)
---
.../DuckDuckGo Privacy Browser.xcscheme | 4 ++
.../Preferences/Model/AboutPreferences.swift | 2 +-
.../View/PreferencesAboutView.swift | 5 --
.../Updates/ReleaseNotesTabExtension.swift | 6 +--
.../Updates/ReleaseNotesUserScript.swift | 2 +-
DuckDuckGo/Updates/Update.swift | 10 ++--
DuckDuckGo/Updates/UpdateController.swift | 47 +++++++++++++------
7 files changed, 49 insertions(+), 27 deletions(-)
diff --git a/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/DuckDuckGo Privacy Browser.xcscheme b/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/DuckDuckGo Privacy Browser.xcscheme
index b272e5bb34..294ba1aeb6 100644
--- a/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/DuckDuckGo Privacy Browser.xcscheme
+++ b/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/DuckDuckGo Privacy Browser.xcscheme
@@ -299,6 +299,10 @@
argument = "-NSConstraintBasedLayoutVisualizeMutuallyExclusiveConstraints YES"
isEnabled = "YES">
+
+
diff --git a/DuckDuckGo/Preferences/Model/AboutPreferences.swift b/DuckDuckGo/Preferences/Model/AboutPreferences.swift
index 63b5433ce8..c664a6bedb 100644
--- a/DuckDuckGo/Preferences/Model/AboutPreferences.swift
+++ b/DuckDuckGo/Preferences/Model/AboutPreferences.swift
@@ -71,7 +71,7 @@ final class AboutPreferences: ObservableObject, PreferencesTabOpening {
#if SPARKLE
func checkForUpdate() {
- updateController?.checkForUpdateIfNeeded()
+ updateController?.checkForUpdateSkippingRollout()
}
func runUpdate() {
diff --git a/DuckDuckGo/Preferences/View/PreferencesAboutView.swift b/DuckDuckGo/Preferences/View/PreferencesAboutView.swift
index 3ba655735a..d6fe08b06a 100644
--- a/DuckDuckGo/Preferences/View/PreferencesAboutView.swift
+++ b/DuckDuckGo/Preferences/View/PreferencesAboutView.swift
@@ -51,11 +51,6 @@ extension Preferences {
}
}
}
- .onAppear {
-#if SPARKLE && !DEBUG
- model.checkForUpdate()
-#endif
- }
}
}
diff --git a/DuckDuckGo/Updates/ReleaseNotesTabExtension.swift b/DuckDuckGo/Updates/ReleaseNotesTabExtension.swift
index 570ea39d05..d258c5e523 100644
--- a/DuckDuckGo/Updates/ReleaseNotesTabExtension.swift
+++ b/DuckDuckGo/Updates/ReleaseNotesTabExtension.swift
@@ -105,11 +105,11 @@ final class ReleaseNotesTabExtension: NavigationResponder {
@MainActor
func navigationDidFinish(_ navigation: Navigation) {
-#if !DEBUG
guard NSApp.runType != .uiTests, navigation.url == .releaseNotes else { return }
let updateController = Application.appDelegate.updateController!
- updateController.checkForUpdateIfNeeded()
-#endif
+ if updateController.latestUpdate?.needsLatestReleaseNote == true {
+ updateController.checkForUpdateSkippingRollout()
+ }
}
}
diff --git a/DuckDuckGo/Updates/ReleaseNotesUserScript.swift b/DuckDuckGo/Updates/ReleaseNotesUserScript.swift
index 1013958aba..ff07147751 100644
--- a/DuckDuckGo/Updates/ReleaseNotesUserScript.swift
+++ b/DuckDuckGo/Updates/ReleaseNotesUserScript.swift
@@ -113,7 +113,7 @@ extension ReleaseNotesUserScript {
@MainActor
private func retryUpdate(params: Any, original: WKScriptMessage) async throws -> Encodable? {
DispatchQueue.main.async { [weak self] in
- self?.updateController.checkForUpdateIfNeeded()
+ self?.updateController.checkForUpdateSkippingRollout()
}
return nil
}
diff --git a/DuckDuckGo/Updates/Update.swift b/DuckDuckGo/Updates/Update.swift
index d36a7b10d1..bfa43a8db6 100644
--- a/DuckDuckGo/Updates/Update.swift
+++ b/DuckDuckGo/Updates/Update.swift
@@ -34,6 +34,7 @@ final class Update {
let date: Date
let releaseNotes: [String]
let releaseNotesPrivacyPro: [String]
+ let needsLatestReleaseNote: Bool
var title: String {
let formatter = DateFormatter()
@@ -47,7 +48,8 @@ final class Update {
build: String,
date: Date,
releaseNotes: [String],
- releaseNotesPrivacyPro: [String]) {
+ releaseNotesPrivacyPro: [String],
+ needsLatestReleaseNote: Bool) {
self.isInstalled = isInstalled
self.type = type
self.version = version
@@ -55,12 +57,13 @@ final class Update {
self.date = date
self.releaseNotes = releaseNotes
self.releaseNotesPrivacyPro = releaseNotesPrivacyPro
+ self.needsLatestReleaseNote = needsLatestReleaseNote
}
}
extension Update {
- convenience init(appcastItem: SUAppcastItem, isInstalled: Bool) {
+ convenience init(appcastItem: SUAppcastItem, isInstalled: Bool, needsLatestReleaseNote: Bool) {
let isCritical = appcastItem.isCriticalUpdate
let version = appcastItem.displayVersionString
let build = appcastItem.versionString
@@ -73,7 +76,8 @@ extension Update {
build: build,
date: date,
releaseNotes: releaseNotes,
- releaseNotesPrivacyPro: releaseNotesPrivacyPro)
+ releaseNotesPrivacyPro: releaseNotesPrivacyPro,
+ needsLatestReleaseNote: needsLatestReleaseNote)
}
}
diff --git a/DuckDuckGo/Updates/UpdateController.swift b/DuckDuckGo/Updates/UpdateController.swift
index e45a117790..cd88d87159 100644
--- a/DuckDuckGo/Updates/UpdateController.swift
+++ b/DuckDuckGo/Updates/UpdateController.swift
@@ -42,7 +42,8 @@ protocol UpdateControllerProtocol: AnyObject {
var lastUpdateCheckDate: Date? { get }
- func checkForUpdateIfNeeded()
+ func checkForUpdateRespectingRollout()
+ func checkForUpdateSkippingRollout()
func runUpdate()
var areAutomaticUpdatesEnabled: Bool { get set }
@@ -64,13 +65,20 @@ final class UpdateController: NSObject, UpdateControllerProtocol {
struct UpdateCheckResult {
let item: SUAppcastItem
let isInstalled: Bool
+ let needsLatestReleaseNote: Bool
+
+ init(item: SUAppcastItem, isInstalled: Bool, needsLatestReleaseNote: Bool = false) {
+ self.item = item
+ self.isInstalled = isInstalled
+ self.needsLatestReleaseNote = needsLatestReleaseNote
+ }
}
private var cachedUpdateResult: UpdateCheckResult?
@Published private(set) var updateProgress = UpdateCycleProgress.default {
didSet {
if let cachedUpdateResult {
- latestUpdate = Update(appcastItem: cachedUpdateResult.item, isInstalled: cachedUpdateResult.isInstalled)
+ latestUpdate = Update(appcastItem: cachedUpdateResult.item, isInstalled: cachedUpdateResult.isInstalled, needsLatestReleaseNote: cachedUpdateResult.needsLatestReleaseNote)
hasPendingUpdate = latestUpdate?.isInstalled == false && updateProgress.isDone && userDriver?.isResumable == true
needsNotificationDot = hasPendingUpdate
}
@@ -101,6 +109,7 @@ final class UpdateController: NSObject, UpdateControllerProtocol {
if oldValue != areAutomaticUpdatesEnabled {
userDriver?.cancelAndDismissCurrentUpdate()
try? configureUpdater()
+ checkForUpdateSkippingRollout()
}
}
}
@@ -129,6 +138,7 @@ final class UpdateController: NSObject, UpdateControllerProtocol {
super.init()
try? configureUpdater()
+ checkForUpdateRespectingRollout()
}
func checkNewApplicationVersion() {
@@ -142,10 +152,18 @@ final class UpdateController: NSObject, UpdateControllerProtocol {
}
}
- func checkForUpdateIfNeeded() {
+ func checkForUpdateRespectingRollout() {
+ guard let updater, !updater.sessionInProgress else { return }
+
+ Logger.updates.log("Checking for updates respecting rollout")
+
+ updater.checkForUpdatesInBackground()
+ }
+
+ func checkForUpdateSkippingRollout() {
guard let updater, !updater.sessionInProgress else { return }
- Logger.updates.log("Checking for updates")
+ Logger.updates.log("Checking for updates skipping rollout")
updater.checkForUpdates()
}
@@ -168,14 +186,6 @@ final class UpdateController: NSObject, UpdateControllerProtocol {
.assign(to: \.updateProgress, onWeaklyHeld: self)
try updater?.start()
-
-#if DEBUG
- updater?.automaticallyChecksForUpdates = false
- updater?.automaticallyDownloadsUpdates = false
- updater?.updateCheckInterval = 0
-#else
- checkForUpdateIfNeeded()
-#endif
}
private func showUpdateNotificationIfNeeded() {
@@ -249,12 +259,18 @@ extension UpdateController: SPUUpdaterDelegate {
func updaterDidNotFindUpdate(_ updater: SPUUpdater, error: any Error) {
let nsError = error as NSError
- guard let item = nsError.userInfo["SULatestAppcastItemFound"] as? SUAppcastItem else { return }
+ guard let item = nsError.userInfo[SPULatestAppcastItemFoundKey] as? SUAppcastItem else { return }
Logger.updates.log("Updater did not find update: \(String(describing: item.displayVersionString))(\(String(describing: item.versionString)))")
PixelKit.fire(DebugEvent(GeneralPixel.updaterDidNotFindUpdate, error: error))
- cachedUpdateResult = UpdateCheckResult(item: item, isInstalled: true)
+ // Edge case: User upgrades to latest version within their rollout group
+ // But fetched release notes are outdated due to rollout group reset
+ let needsLatestReleaseNote = {
+ guard let reason = nsError.userInfo[SPUNoUpdateFoundReasonKey] as? Int else { return false }
+ return reason == Int(Sparkle.SPUNoUpdateFoundReason.onNewerThanLatestVersion.rawValue)
+ }()
+ cachedUpdateResult = UpdateCheckResult(item: item, isInstalled: true, needsLatestReleaseNote: needsLatestReleaseNote)
}
func updater(_ updater: SPUUpdater, didDownloadUpdate item: SUAppcastItem) {
@@ -274,6 +290,9 @@ extension UpdateController: SPUUpdaterDelegate {
if error == nil {
Logger.updates.log("Updater did finish update cycle")
updateProgress = .updateCycleDone
+ } else if let errorCode = (error as? NSError)?.code, errorCode == Int(Sparkle.SUError.noUpdateError.rawValue) {
+ Logger.updates.log("Updater did finish update cycle with no update found")
+ updateProgress = .updateCycleDone
} else {
Logger.updates.log("Updater did finish update cycle with error")
}