Skip to content
This repository was archived by the owner on Dec 14, 2021. It is now read-only.

implement sync timeout #1045

Merged
merged 7 commits into from
Jun 6, 2019
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
2 changes: 2 additions & 0 deletions Shared/Common/Resources/BaseConstants.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public let isRunningTest = NSClassFromString("XCTestCase") != nil
class Constant {
class app {
static let group = "group.org.mozilla.ios.Lockbox"
static let syncTimeout: Double = 20
}

struct fxa {
Expand Down Expand Up @@ -72,6 +73,7 @@ class Constant {
static let usernamePlaceholder = NSLocalizedString("username_placeholder", value: "(no username)", comment: "Placeholder text when there is no username. String should include appropriate open/close parenthetical or similar symbols to indicate this is a placeholder, not a real username.")
static let searchYourEntries = NSLocalizedString("search.placeholder", value: "Search logins", comment: "Placeholder text for search field")
static let emptyListPlaceholder = NSLocalizedString("list.empty", value: "%@ lets you access passwords you’ve already saved to Firefox. To view your logins here, you’ll need to sign in and sync with Firefox.", comment: "Label shown when there are no logins to list. %@ will be replaced with the application name")
static let syncTimedOut = NSLocalizedString("sync.timeout", value: "Sync timed out", comment: "This is the message displayed when syncing entries from the server times out")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there anyone we need to inform of new/changed localized strings?

}
}

Expand Down
22 changes: 16 additions & 6 deletions Shared/Store/BaseDataStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,16 @@ enum SyncError: Error {
}

enum SyncState: Equatable {
case Syncing, Synced
case Syncing, Synced, TimedOut

public static func ==(lhs: SyncState, rhs: SyncState) -> Bool {
switch (lhs, rhs) {
case (Syncing, Syncing):
return true
case (Synced, Synced):
return true
case (TimedOut, TimedOut):
return true
default:
return false
}
Expand Down Expand Up @@ -71,7 +73,7 @@ class BaseDataStore {

internal var disposeBag = DisposeBag()
private var listSubject = BehaviorRelay<[LoginRecord]>(value: [])
private var syncSubject = ReplaySubject<SyncState>.create(bufferSize: 1)
private var syncSubject = BehaviorRelay<SyncState>(value: .Synced)
private var storageStateSubject = ReplaySubject<LoginStoreState>.create(bufferSize: 1)

private let dispatcher: Dispatcher
Expand Down Expand Up @@ -229,21 +231,29 @@ extension BaseDataStore {
else { return }

if (networkStore.isConnectedToNetwork) {
self.syncSubject.onNext(SyncState.Syncing)
self.syncSubject.accept(SyncState.Syncing)
} else {
self.syncSubject.onNext(SyncState.Synced)
self.syncSubject.accept(SyncState.Synced)
return
}

queue.async {
self.queue.asyncAfter(deadline: .now() + Constant.app.syncTimeout, execute: {
// this block serves to "cancel" the sync if the operation is running slowly
if (self.syncSubject.value != .Synced) {
self.syncSubject.accept(.TimedOut)
self.dispatcher.dispatch(action: SentryAction(title: "Sync timeout without error", error: nil, function: "", line: ""))
}
})

do {
try self.loginsStorage?.sync(unlockInfo: syncInfo)
} catch let error as LoginStoreError {
self.storageStateSubject.onNext(.Errored(cause: error))
} catch let error {
NSLog("Unknown error syncing: \(error)")
}
self.syncSubject.onNext(SyncState.Synced)
self.syncSubject.accept(SyncState.Synced)
}
}

Expand Down
2 changes: 1 addition & 1 deletion lockbox-ios/Action/SentryAction.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import Foundation

struct SentryAction: Action {
let title: String
let error: Error
let error: Error?
let function: String
let line: String
}
13 changes: 11 additions & 2 deletions lockbox-ios/Presenter/ItemListPresenter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import RxSwift
import RxCocoa
import RxDataSources

protocol ItemListViewProtocol: AlertControllerView, SpinnerAlertView, BaseItemListViewProtocol {
protocol ItemListViewProtocol: AlertControllerView, StatusAlertView, SpinnerAlertView, BaseItemListViewProtocol {
func bind(sortingButtonTitle: Driver<String>)
func bind(scrollAction: Driver<ScrollAction>)
var sortingButtonEnabled: AnyObserver<Bool>? { get }
Expand Down Expand Up @@ -174,7 +174,7 @@ extension ItemListPresenter {
fileprivate func setupSpinnerDisplay() {
// when this observable emits an event, the spinner gets dismissed
let hideSpinnerObservable = self.dataStore.syncState
.filter { $0 == SyncState.Synced }
.filter { $0 == SyncState.Synced || $0 == SyncState.TimedOut }
.map { _ in return () }
.asDriver(onErrorJustReturn: ())

Expand All @@ -194,6 +194,15 @@ extension ItemListPresenter {
}
})
.disposed(by: self.disposeBag)

self.dataStore.syncState
.filter { $0 == SyncState.TimedOut }
.map { _ in () }
.asDriver(onErrorJustReturn: () )
.drive(onNext: { _ in
self.view?.displayTemporaryAlert(Constant.string.syncTimedOut, timeout: 5)
})
.disposed(by: self.disposeBag)
}
}

Expand Down
2 changes: 1 addition & 1 deletion lockbox-ios/Store/SentryStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class SentryStore {
.subscribe(onNext: { (action) in
Client.shared?.reportUserException(
action.title,
reason: action.error.localizedDescription,
reason: action.error?.localizedDescription ?? "",
language: NSLocale.preferredLanguages.first ?? "",
lineOfCode: action.line,
stackTrace: Thread.callStackSymbols,
Expand Down
2 changes: 1 addition & 1 deletion lockbox-iosTests/BaseDataStoreSpec.swift
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ class BaseDataStoreSpec: QuickSpec {
let syncStates: [SyncState] = self.syncObserver.events.map {
$0.value.element!
}
expect(syncStates).to(equal([SyncState.Synced, SyncState.Syncing, SyncState.Synced]))
expect(syncStates).to(equal([SyncState.Synced, SyncState.Synced, SyncState.Syncing, SyncState.Synced]))
}
}

Expand Down
18 changes: 16 additions & 2 deletions lockbox-iosTests/ItemListPresenterSpec.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ class ItemListPresenterSpec: QuickSpec {
var fakeOnSettingsPressed = PublishSubject<Void>()
var fakeOnSortingButtonPressed = PublishSubject<Void>()
var setFilterEnabledValue: Bool?

var displayOptionSheetButtons: [AlertActionButtonConfiguration]?
var displayOptionSheetTitle: String?
var temporaryAlertArgument: String?

var displayOptionSheetTitle: String?
func bind(items: Driver<[ItemSectionModel]>) {
items.drive(itemsObserver).disposed(by: self.disposeBag)
}
Expand All @@ -53,6 +53,10 @@ class ItemListPresenterSpec: QuickSpec {
self.displayOptionSheetTitle = title
}

func displayTemporaryAlert(_ message: String, timeout: TimeInterval) {
self.temporaryAlertArgument = message
}

func dismissKeyboard() {
self.dismissKeyboardCalled = true
}
Expand Down Expand Up @@ -284,6 +288,16 @@ class ItemListPresenterSpec: QuickSpec {
}
}
}

describe("if the sync times out") {
beforeEach {
self.dataStore.syncStateStub.onNext(SyncState.TimedOut)
}

it("displays a temporary alert for the user") {
expect(self.view.temporaryAlertArgument).to(equal(Constant.string.syncTimedOut))
}
}
}

describe("manual sync") {
Expand Down