slim down syncstate, more datastore tests (#956)

* slim down syncstate, more datastore tests * enum nit * update with deleted item
mozilla-lockwise · May 9, 2019 · e3cc957 · e3cc957
1 parent 4859d39
commit e3cc957
Show file tree

Hide file tree

Showing 7 changed files with 84 additions and 24 deletions.
diff --git a/LockboxXCUITests/LockboxXCUITests.swift b/LockboxXCUITests/LockboxXCUITests.swift
@@ -145,9 +145,9 @@ class LockboxXCUITests: BaseTestCase {
         // There should be the correct number of matches
         let aMatches = app.tables.cells.count
         if  iPad() {
-            XCTAssertEqual(aMatches, 111)
+            XCTAssertEqual(aMatches, 110)
         } else {
-            XCTAssertEqual(aMatches, 108)
+            XCTAssertEqual(aMatches, 107)
         }
         // There should be less number of matches
         searchTextField.typeText("cc")

diff --git a/Shared/Presenter/BaseItemListPresenter.swift b/Shared/Presenter/BaseItemListPresenter.swift
@@ -172,7 +172,7 @@ extension BaseItemListPresenter {
             }
             .distinctUntilChanged()
             .map { (latest: LoginListTextSort) -> [ItemSectionModel] in
-                if (latest.syncState == .Syncing || latest.syncState == .ReadyToSync) && latest.logins.isEmpty {
+                if latest.syncState == .Syncing && latest.logins.isEmpty {
                     return self.syncPlaceholderItems
                 }
 

diff --git a/Shared/Store/BaseDataStore.swift b/Shared/Store/BaseDataStore.swift
@@ -24,20 +24,14 @@ enum SyncError: Error {
 }
 
 enum SyncState: Equatable {
-    case NotSyncable, ReadyToSync, Syncing, Synced, Error(error: SyncError)
+    case Syncing, Synced
 
     public static func ==(lhs: SyncState, rhs: SyncState) -> Bool {
         switch (lhs, rhs) {
-        case (NotSyncable, NotSyncable):
-            return true
-        case (ReadyToSync, ReadyToSync):
-            return true
         case (Syncing, Syncing):
             return true
         case (Synced, Synced):
             return true
-        case (Error, Error):
-            return true
         default:
             return false
         }

diff --git a/Shared/Store/NetworkStore.swift b/Shared/Store/NetworkStore.swift
@@ -7,7 +7,7 @@ import RxSwift
 import RxCocoa
 import Reachability
 
-class NetworkStore {
+open class NetworkStore {
     static let shared = NetworkStore()
 
     private var reachability: ReachabilityProtocol?
@@ -16,11 +16,11 @@ class NetworkStore {
 
     // not using this Observable anywhere yet, but it will be handy to have
     // if we do any further work around offline mode
-    public var connectedToNetwork: Observable<Bool> {
+    open var connectedToNetwork: Observable<Bool> {
         return _connectedToNetwork.asObservable()
     }
 
-    public var isConnectedToNetwork: Bool {
+    open var isConnectedToNetwork: Bool {
         return _connectedToNetwork.value
     }
 

diff --git a/lockbox-ios/Presenter/ItemListPresenter.swift b/lockbox-ios/Presenter/ItemListPresenter.swift
@@ -186,8 +186,7 @@ extension ItemListPresenter {
                 .asDriver(onErrorJustReturn: SyncStateManual(syncState: .Synced, manualSync: false))
                 .throttle(2.0)
                 .drive(onNext: { latest in
-                    if (latest.syncState == SyncState.Syncing || latest.syncState == SyncState.ReadyToSync)
-                               && !latest.manualSync {
+                    if latest.syncState == .Syncing && !latest.manualSync {
                         self.view?.displaySpinner(hideSpinnerObservable,
                                                   bag: self.disposeBag,
                                                   message: Constant.string.syncingYourEntries,

diff --git a/lockbox-ios/Presenter/RootPresenter.swift b/lockbox-ios/Presenter/RootPresenter.swift
@@ -125,14 +125,10 @@ class RootPresenter {
             })
             .disposed(by: self.disposeBag)
 
-        Observable.combineLatest(self.dataStore.syncState, self.dataStore.storageState)
-            .filter { $0.1 == LoginStoreState.Unprepared }
-            .map { $0.0 }
-            .distinctUntilChanged()
-            .subscribe(onNext: { syncState in
-                if syncState == .NotSyncable {
-                    self.dispatcher.dispatch(action: LoginRouteAction.welcome)
-                }
+        self.dataStore.storageState
+            .filter { $0 == LoginStoreState.Unprepared }
+            .subscribe(onNext: { _ in
+                self.dispatcher.dispatch(action: LoginRouteAction.welcome)
             })
             .disposed(by: self.disposeBag)
 

diff --git a/lockbox-iosTests/BaseDataStoreSpec.swift b/lockbox-iosTests/BaseDataStoreSpec.swift
@@ -145,6 +145,14 @@ class BaseDataStoreSpec: QuickSpec {
         }
     }
 
+    class FakeNetworkStore: NetworkStore {
+        var connectedStub = false
+
+        override var isConnectedToNetwork: Bool {
+            return self.connectedStub
+        }
+    }
+
     class FakeLifecycleStore: LifecycleStore {
         var fakeCycle = PublishSubject<LifecycleAction>()
 
@@ -158,7 +166,7 @@ class BaseDataStoreSpec: QuickSpec {
                       keychainWrapper: KeychainWrapper,
                       autoLockSupport: AutoLockSupport,
                       dataStoreSupport: DataStoreSupport,
-                      networkStore: NetworkStore = NetworkStore.shared,
+                      networkStore: NetworkStore,
                       lifecycleStore: LifecycleStore) {
             super.init(
                     dispatcher: dispatcher,
@@ -199,6 +207,7 @@ class BaseDataStoreSpec: QuickSpec {
     private var autoLockSupport: FakeAutoLockSupport!
     private var dataStoreSupport: FakeDataStoreSupport!
     private var lifecycleStore: FakeLifecycleStore!
+    private var networkStore: FakeNetworkStore!
     var subject: BaseDataStore!
 
     override func spec() {
@@ -210,11 +219,13 @@ class BaseDataStoreSpec: QuickSpec {
                 self.autoLockSupport = FakeAutoLockSupport()
                 self.dataStoreSupport = FakeDataStoreSupport(loginsStorageStub: self.loginsStorage)
                 self.lifecycleStore = FakeLifecycleStore()
+                self.networkStore = FakeNetworkStore()
                 self.subject = FakeDataStoreImpl(
                         dispatcher: self.dispatcher,
                         keychainWrapper: self.keychainWrapper,
                         autoLockSupport: self.autoLockSupport,
                         dataStoreSupport: self.dataStoreSupport,
+                        networkStore: self.networkStore,
                         lifecycleStore: self.lifecycleStore
                 )
 
@@ -352,6 +363,56 @@ class BaseDataStoreSpec: QuickSpec {
                 }
             }
 
+            describe("sync") {
+                beforeEach {
+                    let syncCred = SyncCredential(syncInfo: self.syncInfo, isNew: true)
+                    self.dispatcher.dispatch(action: DataStoreAction.updateCredentials(syncInfo: syncCred))
+                }
+
+                describe("when the network is available") {
+                    beforeEach {
+                        self.networkStore.connectedStub = true
+                        self.dispatcher.dispatch(action: DataStoreAction.sync)
+                    }
+
+                    it("syncs + pushes syncing followed by synced") {
+                        expect(self.loginsStorage.syncArgument).notTo(beNil())
+                        let _ = try! self.subject.syncState.toBlocking().first()
+                        let syncStates: [SyncState] = self.syncObserver.events.map {
+                            $0.value.element!
+                        }
+                        expect(syncStates).to(equal([SyncState.Synced, SyncState.Syncing, SyncState.Synced]))
+                    }
+                }
+
+                describe("when the network is not available") {
+                    beforeEach {
+                        self.networkStore.connectedStub = false
+                        self.dispatcher.dispatch(action: DataStoreAction.sync)
+                    }
+
+                    it("pushes synced and does nothing else") {
+                        let state = try! self.subject.syncState.toBlocking().first()
+                        expect(state).to(equal(SyncState.Synced))
+                        expect(self.loginsStorage.syncArgument).to(beNil())
+                    }
+                }
+            }
+
+            describe("touch") {
+                let id = "lieksmxhwmldkdmsldlf"
+
+                beforeEach {
+                    let syncCred = SyncCredential(syncInfo: self.syncInfo, isNew: true)
+                    self.dispatcher.dispatch(action: DataStoreAction.updateCredentials(syncInfo: syncCred))
+                    self.dispatcher.dispatch(action: DataStoreAction.touch(id: id))
+                }
+
+                it("touches the item in the datastore") {
+                    expect(self.loginsStorage.touchIdArgument).to(equal(id))
+                }
+            }
+
             describe("lifecycle interactions") {
                 describe("background events") {
                     beforeEach {
@@ -387,5 +448,15 @@ class BaseDataStoreSpec: QuickSpec {
                 }
             }
         }
+
+        describe("syncstate") {
+            describe("equality") {
+                it("same states are equal") {
+                    expect(SyncState.Syncing).to(equal(SyncState.Syncing))
+                    expect(SyncState.Synced).to(equal(SyncState.Synced))
+                    expect(SyncState.Synced).notTo(equal(SyncState.Syncing))
+                }
+            }
+        }
     }
 }