My Profile Network logic

Podfile

@@ -56,6 +56,7 @@ target :WordPressTest, :exclusive => true do
   pod 'Expecta', '0.3.2'
   pod 'Nimble', '~> 3.0.0'
   pod 'RxSwift', '~> 2.1.0'
+  pod 'RxTests', '~> 2.1.0'
 end
 
 target 'UITests', :exclusive => true do

Podfile.lock

@@ -138,6 +138,8 @@ PODS:
   - RxCocoa (2.1.0):
     - RxSwift (~> 2.0)
   - RxSwift (2.1.0)
+  - RxTests (2.1.0):
+    - RxSwift (~> 2.0)
   - Simperium (0.8.10):
     - Simperium/DiffMatchPach (= 0.8.10)
     - Simperium/JRSwizzle (= 0.8.10)
@@ -210,6 +212,7 @@ DEPENDENCIES:
   - ReactiveCocoa (~> 2.4.7)
   - RxCocoa (~> 2.1.0)
   - RxSwift (~> 2.1.0)
+  - RxTests (~> 2.1.0)
   - Simperium (= 0.8.10)
   - Specta (= 1.0.5)
   - SVProgressHUD (~> 1.1.3)
@@ -282,6 +285,7 @@ SPEC CHECKSUMS:
   ReactiveCocoa: eb38dee0a0e698f73a9b25e5c1faea2bb4c79240
   RxCocoa: 79b5feb8378545336e756a0a33fcf5e95050b71c
   RxSwift: 110fb07f81c17c2c3b3254d168363057b1880d18
+  RxTests: 94c67ffc37c36bd8c7aec90a84601a3db142be94
   Simperium: f507d9b400c499048a98fe728a0b2b9956fd14c1
   Specta: ac94d110b865115fe60ff2c6d7281053c6f8e8a2
   SVProgressHUD: 748080e4f36e603f6c02aec292664239df5279c1

WordPress/Classes/Extensions/Rx.swift

@@ -0,0 +1,83 @@
+import RxSwift
+
+// MARK: - pausable
+
+extension ObservableType {
+
+    /**
+     Pauses the underlying observable sequence based upon the observable sequence which yields true/false.
+
+     - parameter pauser: The observable sequence used to pause the underlying sequence.
+     - returns: An observable sequence that subscribes and emits the values of the source observable as long as the last emitted value of the condition observable is true.
+     */
+    public func pausable<ConditionO: ObservableConvertibleType where ConditionO.E == Bool>(pauser: ConditionO) -> Observable<E> {
+        return Pausable(source: self, pauser: pauser.asObservable()).asObservable()
+    }
+}
+
+class Pausable<S: ObservableType>: ObservableType {
+    typealias E = S.E
+    typealias DisposeKey = CompositeDisposable.DisposeKey
+
+    private let _lock = NSRecursiveLock()
+
+    private let _source: S
+    private let _pauser: Observable<Bool>
+    private let _group = CompositeDisposable()
+
+    private var _connectionKey: DisposeKey? = nil
+
+    init(source: S, pauser: Observable<Bool>) {
+        _source = source
+        _pauser = pauser
+    }
+
+    func subscribe<O: ObserverType where O.E == E>(observer: O) -> Disposable {
+        let conn = _source.publish()
+        let connection = conn.subscribe(observer)
+        _group.addDisposable(connection)
+
+        let subscription = _pauser
+            .distinctUntilChanged()
+            .subscribeNext { active in
+                self._lock.lock(); defer { self._lock.unlock() } // lock {
+                    if active {
+                        self._connectionKey = self._group.addDisposable(conn.connect())
+                    } else {
+                        if let connectionKey = self._connectionKey {
+                            self._group.removeDisposable(connectionKey)
+                            self._connectionKey = nil
+                        }
+                    }
+                // }
+        }
+        _group.addDisposable(subscription)
+
+        return _group
+    }
+}
+
+// MARK: - retryIf
+
+extension ObservableType {
+    /**
+     Repeats the source observable sequence on error if the given condition evaluates true.
+
+     - parameter condition: A closure to be evaluated on error to decide if the source sequence should be retried. It takes two parameters: an incrementing `count` integer, and a `lastError` containing the latest error emitted.
+     - returns: An observable sequence producing the elements of the given sequence repeatedly until it terminates successfully or the condition evaluates false.
+     */
+    public func retryIf(condition: (count: Int, lastError: NSError) -> Bool) -> Observable<E> {
+        return retryWhen { (errors: Observable<NSError>) in
+            errors.scan((0, nil)) { (accumulator: (Int, NSError!), error) in
+                (accumulator.0 + 1, error)
+            }
+            .flatMap { (count, lastError) -> Observable<Int> in
+                if condition(count: count, lastError: lastError) {
+                    return Observable.just(count)
+                } else {
+                    return Observable.error(lastError)
+                }
+            }
+        }
+    }
+}

WordPress/Classes/Networking/AccountSettingsRemote.swift

@@ -1,12 +1,78 @@
+import AFNetworking
 import Foundation
+import RxSwift
 
 class AccountSettingsRemote: ServiceRemoteREST {
-    func getSettings(success success: AccountSettings -> Void, failure: ErrorType -> Void) {
+    static let remotes = NSMapTable(keyOptions: .StrongMemory, valueOptions: .WeakMemory)
+
+    /// Returns an AccountSettingsRemote with the given api, reusing a previous
+    /// remote if it exists.
+    static func remoteWithApi(api: WordPressComApi) -> AccountSettingsRemote {
+        // We're hashing on the authToken because we don't want duplicate api
+        // objects for the same account.
+        //
+        // In theory this would be taken care of by the fact that the api comes
+        // from a WPAccount, and since WPAccount is a managed object Core Data
+        // guarantees there's only one of it.
+        // 
+        // However it might be possible that the account gets deallocated and
+        // when it's fetched again it would create a different api object.
+        let key = api.authToken.hashValue
+        // FIXME: not thread safe
+        // @koke 2016-01-21
+        if let remote = remotes.objectForKey(key) {
+            return remote as! AccountSettingsRemote
+        } else {
+            let remote = AccountSettingsRemote(api: api)
+            remotes.setObject(remote, forKey: key)
+            return remote
+        }
+    }
+
+    let settings: Observable<AccountSettings>
+
+    /// Creates a new AccountSettingsRemote. It is recommended that you use AccountSettingsRemote.remoteWithApi(_)
+    /// instead.
+    override init(api: WordPressComApi) {
+        settings = AccountSettingsRemote.settingsWithApi(api)
+        super.init(api: api)
+    }
+
+    private static func settingsWithApi(api: WordPressComApi) -> Observable<AccountSettings> {
+        let settings = Observable<AccountSettings>.create { observer in
+            let remote = AccountSettingsRemote(api: api)
+            let operation = remote.getSettings(
+                success: { settings in
+                    observer.onNext(settings)
+                    observer.onCompleted()
+                }, failure: { error in
+                    let nserror = error as NSError
+                    if nserror.domain == NSURLErrorDomain && nserror.code == NSURLErrorCancelled {
+                        // If we canceled the operation, don't propagate the error
+                        // This probably means the observable is being disposed
+                        DDLogSwift.logError("Canceled refreshing settings")
+                    } else {
+                        observer.onError(error)
+                    }
+            })
+            return AnonymousDisposable() {
+                if let operation = operation {
+                    if !operation.finished {
+                        operation.cancel()
+                    }
+                }
+            }
+        }
+
+        return settings
+    }
+
+    func getSettings(success success: AccountSettings -> Void, failure: ErrorType -> Void) -> AFHTTPRequestOperation? {
         let endpoint = "me/settings"
         let parameters = ["context": "edit"]
         let path = pathForEndpoint(endpoint, withVersion: ServiceRemoteRESTApiVersion_1_1)
 
-        api.GET(path,
+        return api.GET(path,
             parameters: parameters,
             success: {
                 operation, responseObject in
@@ -85,4 +151,4 @@ class AccountSettingsRemote: ServiceRemoteREST {
     enum Error: ErrorType {
         case DecodeError
     }
-}
+}

WordPress/Classes/Services/AccountSettingsService.swift

@@ -1,34 +1,127 @@
 import Foundation
+import Reachability
 import RxCocoa
 import RxSwift
 
 let AccountSettingsServiceChangeSaveFailedNotification = "AccountSettingsServiceChangeSaveFailed"
 
-struct AccountSettingsService {
-    let remote: AccountSettingsRemote
+protocol AccountSettingsRemoteInterface {
+    var settings: Observable<AccountSettings> { get }
+    func updateSetting(change: AccountSettingsChange, success: () -> Void, failure: ErrorType -> Void)
+}
+
+extension AccountSettingsRemote: AccountSettingsRemoteInterface {}
+
+class AccountSettingsService {
+    struct Defaults {
+        static let stallTimeout = 4.0
+        static let maxRetries = 3
+        static let pollingInterval = 60.0
+    }
+
+    let remote: AccountSettingsRemoteInterface
     let userID: Int
 
     private let context = ContextManager.sharedInstance().mainContext
 
-    init(userID: Int, api: WordPressComApi) {
-        self.remote = AccountSettingsRemote(api: api)
-        self.userID = userID
+    var testScheduler: SchedulerType? = nil
+    private var scheduler: SchedulerType {
+        return testScheduler ?? MainScheduler.instance
     }
 
-    func refreshSettings(completion: (Bool) -> Void) {
-        remote.getSettings(
-            success: {
-                (settings) -> Void in
+    convenience init(userID: Int, api: WordPressComApi) {
+        let remote = AccountSettingsRemote.remoteWithApi(api)
+        self.init(userID: userID, remote: remote)
+    }
 
-                self.updateSettings(settings)
-                completion(true)
-            }, failure: {
-                (error) -> Void in
+    init(userID: Int, remote: AccountSettingsRemoteInterface) {
+        self.userID = userID
+        self.remote = remote
+    }
 
-                DDLogSwift.logError(String(error))
-                completion(false)
+    var testReachability: Observable<Bool>? = nil
+    /// Emits a boolean value each time reachability changes for the internet connection.
+    private lazy var reachable: Observable<Bool> = {
+        return self.testReachability ?? Reachability.internetConnection
+    }()
+
+    /// Performs a network refresh of settings and emits values with the refresh status.
+    ///
+    /// - When it's subscribed, it requests a refresh from the server
+    /// - If a networking error happens it doesn't emit a new value and will retry the request.
+    /// - If it reaches the maximum permitted number of retries it will emit an Error.
+    /// - If an error not related to networking happens, it will emit an Error.
+    /// - When the data is refreshed, it will emit an `.Idle` value and complete.
+    private lazy var remoteSettings: Observable<RefreshStatus> = {
+        return self.remote.settings
+            .map({ settings -> RefreshStatus in
+                self.updateSettings(settings)
+                return .Idle
+            })
+            .retryIf({ (count, error) in
+                if error.domain == NSURLErrorDomain {
+                    DDLogSwift.logError("Error refreshing settings (attempt \(count)): \(error)")
+                } else {
+                    DDLogSwift.logError("Error refreshing settings (unrecoverable): \(error)")
+                }
+
+                return error.domain == NSURLErrorDomain && count < Defaults.maxRetries
+            })
+    }()
+
+    /// Emits one `.Stalled` value after a timeout and then completes
+    private lazy var stalled: Observable<RefreshStatus> = {
+        return Observable<RefreshStatus>
+            .just(.Stalled)
+            .delaySubscription(Defaults.stallTimeout, scheduler: self.scheduler)
+    }()
+
+    /// Performs a network refresh of settings and emits values with the refresh status.
+    ///
+    /// - When it's subscribed, it requests a refresh from the server
+    /// - If it takes more than `stallTimeout` to complete, it will emit a `.Stalled` value and continue waiting for the request to finish.
+    /// - If a networking error happens it doesn't emit a new value and will retry the request.
+    /// - If it reaches the maximum permitted number of retries it will emit an Error.
+    /// - If an error not related to networking happens, it will emit an Error.
+    /// - When the data is refreshed, it will emit an `.Idle` value and complete.
+    lazy private(set) var request: Observable<RefreshStatus> = {
+        let remoteSettings = self.remoteSettings.shareReplayLatestWhileConnected()
+        let stalledSettings = Observable.of(self.stalled, remoteSettings)
+            .merge()
+
+        return remoteSettings
+            .amb(stalledSettings)
+            .startWith(.Refreshing)
+    }()
+
+    /// Emits values when the refresh status changes.
+    ///
+    /// On subscription, this will start refreshing settings, polling each minute, while there's an internet connection.
+    /// Possible values:
+    /// - `.Refreshing` when it starts getting remote data.
+    /// - `.Stalled` when it's getting remote data and hasn't succeeded before `stallTimeout`.
+    /// - `.Offline` when there is no internet connection.
+    /// - `.Idle` when the request was successful and it's waiting for the polling interval.
+    /// - An error when the request couldn't complete. It will stop retrying.
+    lazy var refresh: Observable<RefreshStatus> = {
+        // Copy request to avoid capture of self in closure
+        let request = self.request
+
+        // Convert to a polling request
+        let polling = Observable<Int>
+            .interval(Defaults.pollingInterval, scheduler: self.scheduler)
+            .startWith(0)
+            .flatMapLatest({ _ in request })
+
+        // Enable only when reachable, otherwise emit .Offline
+        return self.reachable.flatMapLatest({ reachable -> Observable<RefreshStatus> in
+            if reachable {
+                return polling
+            } else {
+                return Observable.just(.Offline)
+            }
         })
-    }
+    }()
 
     func saveChange(change: AccountSettingsChange) {
         guard let reverse = try? applyChange(change) else {
@@ -113,4 +206,25 @@ struct AccountSettingsService {
     enum Errors: ErrorType {
         case NotFound
     }
+
+    enum RefreshStatus {
+        case Idle
+        case Refreshing
+        case Stalled
+        case Failed
+        case Offline
+
+        var errorMessage: String? {
+            switch self {
+            case Stalled:
+                return NSLocalizedString("We are having trouble loading data", comment: "Error message displayed when a refresh is taking longer than usual. The refresh hasn't failed and it might still succeed")
+            case Failed:
+                return NSLocalizedString("We had trouble loading data", comment: "Error message displayed when a refresh failed")
+            case Offline:
+                return NSLocalizedString("You are currently offline", comment: "Error message displayed when the app can't connect to the API servers")
+            case Idle, Refreshing:
+                return nil
+            }
+        }
+    }
 }