Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sync limit exceeded #2105

Merged
merged 3 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Core/PixelEvent.swift
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,10 @@ extension Pixel {
case syncFailedToMigrate
case syncFailedToLoadAccount
case syncFailedToSetupEngine
case syncBookmarksCountLimitExceededDaily
case syncCredentialsCountLimitExceededDaily
case syncBookmarksRequestSizeLimitExceededDaily
case syncCredentialsRequestSizeLimitExceededDaily

case syncSentUnauthenticatedRequest
case syncMetadataCouldNotLoadDatabase
Expand Down Expand Up @@ -952,6 +956,10 @@ extension Pixel.Event {
case .syncFailedToMigrate: return "m_d_sync_failed_to_migrate"
case .syncFailedToLoadAccount: return "m_d_sync_failed_to_load_account"
case .syncFailedToSetupEngine: return "m_d_sync_failed_to_setup_engine"
case .syncBookmarksCountLimitExceededDaily: return "m_d_sync_bookmarks_count_limit_exceeded_daily"
case .syncCredentialsCountLimitExceededDaily: return "m_d_sync_credentials_count_limit_exceeded_daily"
case .syncBookmarksRequestSizeLimitExceededDaily: return "m_d_sync_bookmarks_request_size_limit_exceeded_daily"
case .syncCredentialsRequestSizeLimitExceededDaily: return "m_d_sync_credentials_request_size_limit_exceeded_daily"

case .syncSentUnauthenticatedRequest: return "m_d_sync_sent_unauthenticated_request"
case .syncMetadataCouldNotLoadDatabase: return "m_d_sync_metadata_could_not_load_database"
Expand Down
19 changes: 19 additions & 0 deletions Core/SyncBookmarksAdapter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@ public final class SyncBookmarksAdapter {
public let databaseCleaner: BookmarkDatabaseCleaner
public let syncDidCompletePublisher: AnyPublisher<Void, Never>
public let widgetRefreshCancellable: AnyCancellable
public static let syncBookmarksPausedStateChanged = Notification.Name("com.duckduckgo.app.SyncPausedStateChanged")

@UserDefaultsWrapper(key: .syncBookmarksPaused, defaultValue: false)
static private var isSyncBookmarksPaused: Bool {
didSet {
NotificationCenter.default.post(name: syncBookmarksPausedStateChanged, object: nil)
}
}

public init(database: CoreDataDatabase, favoritesDisplayModeStorage: FavoritesDisplayModeStoring) {
self.database = database
Expand Down Expand Up @@ -71,6 +79,7 @@ public final class SyncBookmarksAdapter {
metadataStore: metadataStore,
syncDidUpdateData: { [syncDidCompleteSubject] in
syncDidCompleteSubject.send()
Self.isSyncBookmarksPaused = false
}
)

Expand All @@ -79,6 +88,16 @@ public final class SyncBookmarksAdapter {
switch error {
case let syncError as SyncError:
Pixel.fire(pixel: .syncBookmarksFailed, error: syncError)
// If bookmarks count limit has been exceeded
if syncError == .unexpectedStatusCode(409) {
Self.isSyncBookmarksPaused = true
DailyPixel.fire(pixel: .syncBookmarksCountLimitExceededDaily)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could use else here, or rewrite if statements as switch/case statement.

// If bookmarks request size limit has been exceeded
if syncError == .unexpectedStatusCode(413) {
Self.isSyncBookmarksPaused = true
DailyPixel.fire(pixel: .syncBookmarksRequestSizeLimitExceededDaily)
}
default:
let nsError = error as NSError
if nsError.domain != NSURLErrorDomain {
Expand Down
19 changes: 19 additions & 0 deletions Core/SyncCredentialsAdapter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ public final class SyncCredentialsAdapter {
public private(set) var provider: CredentialsProvider?
public let databaseCleaner: CredentialsDatabaseCleaner
public let syncDidCompletePublisher: AnyPublisher<Void, Never>
public static let syncCredentialsPausedStateChanged = SyncBookmarksAdapter.syncBookmarksPausedStateChanged

@UserDefaultsWrapper(key: .syncCredentialsPaused, defaultValue: false)
static private var isSyncCredentialsPaused: Bool {
didSet {
NotificationCenter.default.post(name: syncCredentialsPausedStateChanged, object: nil)
}
}

public init(secureVaultFactory: AutofillVaultFactory = AutofillSecureVaultFactory, secureVaultErrorReporter: SecureVaultErrorReporting) {
syncDidCompletePublisher = syncDidCompleteSubject.eraseToAnyPublisher()
Expand Down Expand Up @@ -63,6 +71,7 @@ public final class SyncCredentialsAdapter {
metadataStore: metadataStore,
syncDidUpdateData: { [weak self] in
self?.syncDidCompleteSubject.send()
Self.isSyncCredentialsPaused = false
}
)

Expand All @@ -71,6 +80,16 @@ public final class SyncCredentialsAdapter {
switch error {
case let syncError as SyncError:
Pixel.fire(pixel: .syncCredentialsFailed, error: syncError)
// If credentials count limit has been exceeded
if syncError == .unexpectedStatusCode(409) {
Self.isSyncCredentialsPaused = true
DailyPixel.fire(pixel: .syncCredentialsCountLimitExceededDaily)
}
// If credentials request size limit has been exceeded
if syncError == .unexpectedStatusCode(413) {
Self.isSyncCredentialsPaused = true
DailyPixel.fire(pixel: .syncCredentialsRequestSizeLimitExceededDaily)
}
default:
let nsError = error as NSError
if nsError.domain != NSURLErrorDomain {
Expand Down
2 changes: 2 additions & 0 deletions Core/UserDefaultsPropertyWrapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ public struct UserDefaultsWrapper<T> {
case defaultBrowserUsageLastSeen = "com.duckduckgo.ios.default-browser-usage-last-seen"

case syncEnvironment = "com.duckduckgo.ios.sync-environment"
case syncBookmarksPaused = "com.duckduckgo.ios.sync-bookmarksPaused"
case syncCredentialsPaused = "com.duckduckgo.ios.sync-credentialsPaused"

case networkProtectionDebugOptionAlwaysOnDisabled = "com.duckduckgo.network-protection.always-on.disabled"
}
Expand Down
3 changes: 3 additions & 0 deletions DuckDuckGo/AppSettings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,7 @@ protocol AppSettings: AnyObject {

var autoconsentPromptSeen: Bool { get set }
var autoconsentEnabled: Bool { get set }

var isSyncBookmarksPaused: Bool { get }
var isSyncCredentialsPaused: Bool { get }
}
8 changes: 8 additions & 0 deletions DuckDuckGo/AppUserDefaults.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ public class AppUserDefaults: AppSettings {
public static let currentFireButtonAnimationChange = Notification.Name("com.duckduckgo.app.CurrentFireButtonAnimationChange")
public static let textSizeChange = Notification.Name("com.duckduckgo.app.TextSizeChange")
public static let favoritesDisplayModeChange = Notification.Name("com.duckduckgo.app.FavoritesDisplayModeChange")
public static let syncPausedStateChanged = SyncBookmarksAdapter.syncBookmarksPausedStateChanged
public static let syncCredentialsPausedStateChanged = SyncCredentialsAdapter.syncCredentialsPausedStateChanged
public static let autofillEnabledChange = Notification.Name("com.duckduckgo.app.AutofillEnabledChange")
public static let didVerifyInternalUser = Notification.Name("com.duckduckgo.app.DidVerifyInternalUser")
public static let inspectableWebViewsToggled = Notification.Name("com.duckduckgo.app.DidToggleInspectableWebViews")
Expand Down Expand Up @@ -188,6 +190,12 @@ public class AppUserDefaults: AppSettings {
@UserDefaultsWrapper(key: .textSize, defaultValue: 100)
var textSize: Int

@UserDefaultsWrapper(key: .syncBookmarksPaused, defaultValue: false)
var isSyncBookmarksPaused: Bool

@UserDefaultsWrapper(key: .syncCredentialsPaused, defaultValue: false)
var isSyncCredentialsPaused: Bool

public var favoritesDisplayMode: FavoritesDisplayMode {
get {
guard let string = userDefaults?.string(forKey: Keys.favoritesDisplayMode), let favoritesDisplayMode = FavoritesDisplayMode(string) else {
Expand Down
2 changes: 1 addition & 1 deletion DuckDuckGo/MainViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -876,7 +876,7 @@ class MainViewController: UIViewController {
suggestionTrayController?.didHide()
}

fileprivate func launchAutofillLogins(with currentTabUrl: URL? = nil) {
func launchAutofillLogins(with currentTabUrl: URL? = nil) {
let appSettings = AppDependencyProvider.shared.appSettings
let autofillSettingsViewController = AutofillLoginSettingsListViewController(
appSettings: appSettings,
Expand Down
12 changes: 12 additions & 0 deletions DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,18 @@ import AVFoundation

extension SyncSettingsViewController: SyncManagementViewModelDelegate {

func launchAutofillViewController() {
guard let mainVC = view.window?.rootViewController as? MainViewController else { return }
self.dismiss(animated: true)
mainVC.launchAutofillLogins()
}

func launchBookmarksViewController() {
guard let mainVC = view.window?.rootViewController as? MainViewController else { return }
self.dismiss(animated: true)
mainVC.segueToBookmarks()
}

func updateDeviceName(_ name: String) {
Task { @MainActor in
rootView.model.devices = []
Expand Down
13 changes: 13 additions & 0 deletions DuckDuckGo/SyncSettingsViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class SyncSettingsViewController: UIHostingController<SyncSettingsView> {
self.init(rootView: SyncSettingsView(model: viewModel))

setUpFavoritesDisplayModeSwitch(viewModel, appSettings)
setUpSyncPaused(viewModel, appSettings)
refreshForState(syncService.authState)

syncService.authStatePublisher
Expand Down Expand Up @@ -95,6 +96,18 @@ class SyncSettingsViewController: UIHostingController<SyncSettingsView> {
.store(in: &cancellables)
}

private func setUpSyncPaused(_ viewModel: SyncSettingsViewModel, _ appSettings: AppSettings) {
viewModel.isSyncBookmarksPaused = appSettings.isSyncBookmarksPaused
viewModel.isSyncCredentialsPaused = appSettings.isSyncCredentialsPaused
NotificationCenter.default.publisher(for: AppUserDefaults.Notifications.syncPausedStateChanged)
.receive(on: DispatchQueue.main)
.sink { _ in
viewModel.isSyncBookmarksPaused = appSettings.isSyncBookmarksPaused
viewModel.isSyncCredentialsPaused = appSettings.isSyncCredentialsPaused
}
.store(in: &cancellables)
}

override func viewDidLoad() {
super.viewDidLoad()
applyTheme(ThemeManager.shared.currentTheme)
Expand Down
4 changes: 4 additions & 0 deletions DuckDuckGoTests/AppSettingsMock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ import Foundation
@testable import DuckDuckGo

class AppSettingsMock: AppSettings {

var isSyncBookmarksPaused: Bool = false

var isSyncCredentialsPaused: Bool = false

var autofillCredentialsEnabled: Bool = false

Expand Down
28 changes: 28 additions & 0 deletions DuckDuckGoTests/SyncManagementViewModelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,26 @@ class SyncManagementViewModelTests: XCTestCase, SyncManagementViewModelDelegate
])
}

func testWhenManageBookmarksCalled_BookmarksVCIsLaunched() {
model.manageBookmarks()

// You can either test one individual call was made x number of times or check for a whole number of calls
monitor.assert(#selector(launchBookmarksViewController).description, calls: 1)
monitor.assertCalls([
#selector(launchBookmarksViewController).description: 1
])
}

func testWhenManageLogindCalled_AutofillVCIsLaunched() {
model.manageLogins()

// You can either test one individual call was made x number of times or check for a whole number of calls
monitor.assert(#selector(launchAutofillViewController).description, calls: 1)
monitor.assertCalls([
#selector(launchAutofillViewController).description: 1
])
}

// MARK: Delegate functions

func showSyncWithAnotherDeviceEnterText() {
Expand Down Expand Up @@ -148,6 +168,14 @@ class SyncManagementViewModelTests: XCTestCase, SyncManagementViewModelDelegate
monitor.incrementCalls(function: #function.cleaningFunctionName())
}

func launchBookmarksViewController() {
monitor.incrementCalls(function: #function.cleaningFunctionName())
}

func launchAutofillViewController() {
monitor.incrementCalls(function: #function.cleaningFunctionName())
}

}

// MARK: An idea... can be made more public if works out
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ public protocol SyncManagementViewModelDelegate: AnyObject {
func updateDeviceName(_ name: String)
func refreshDevices(clearDevices: Bool)
func updateOptions()
func launchBookmarksViewController()
func launchAutofillViewController()
}

public class SyncSettingsViewModel: ObservableObject {
Expand Down Expand Up @@ -77,6 +79,8 @@ public class SyncSettingsViewModel: ObservableObject {
@Published public var isFaviconsSyncEnabled = false
@Published public var isUnifiedFavoritesEnabled = true
@Published public var isSyncingDevices = false
@Published public var isSyncBookmarksPaused = false
@Published public var isSyncCredentialsPaused = false

@Published var isBusy = false
@Published var recoveryCode = ""
Expand Down Expand Up @@ -148,4 +152,12 @@ public class SyncSettingsViewModel: ObservableObject {
delegate?.createAccountAndStartSyncing(optionsViewModel: self)
}

public func manageBookmarks() {
delegate?.launchBookmarksViewController()
}

public func manageLogins() {
delegate?.launchAutofillViewController()
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -112,5 +112,12 @@ struct UserText {
return "\"\(name)\" will no longer be able to access your synced data."
}

static let syncLimitExceededTitle = "⚠️ Sync Paused"
static let bookmarksLimitExceededDescription = "Bookmark limit exceeded. Delete some to resume syncing."
static let credentialsLimitExceededDescription = "Logins limit exceeded. Delete some to resume syncing."
static let bookmarksLimitExceededAction = "Manage Bookmarks"
static let credentialsLimitExceededAction = "Manage Logins"


}
// swiftlint:enable line_length
Loading