Move cached users from UserService to DataStore (#23865)

* Move cached users from UserService to DataStore

* Repurpose UserServiceTests to test view model instead

* Add unit tests for InMemoryDataStore

WordPress/Classes/Services/UserService.swift

@@ -8,17 +8,10 @@ import WordPressUI
 actor UserService: UserServiceProtocol {
     private let client: WordPressClient
 
-    private var fetchUsersTask: Task<[DisplayUser], Error>?
-
-    private(set) var users: [DisplayUser]? {
-        didSet {
-            if let users {
-                usersUpdatesContinuation.yield(users)
-            }
-        }
+    private let _dataStore: InMemoryUserDataStore = .init()
+    var dataStore: any UserDataStore {
+        _dataStore
     }
-    nonisolated let usersUpdates: AsyncStream<[DisplayUser]>
-    private nonisolated let usersUpdatesContinuation: AsyncStream<[DisplayUser]>.Continuation
 
     private var _currentUser: UserWithEditContext?
     private var currentUser: UserWithEditContext? {
@@ -32,34 +25,20 @@ actor UserService: UserServiceProtocol {
 
     init(client: WordPressClient) {
         self.client = client
-        (usersUpdates, usersUpdatesContinuation) = AsyncStream<[DisplayUser]>.makeStream()
     }
 
-    deinit {
-        usersUpdatesContinuation.finish()
-        fetchUsersTask?.cancel()
-    }
+    func fetchUsers() async throws {
+        let sequence = await client.api.users.sequenceWithEditContext(params: .init(perPage: 100))
+        var started = false
+        for try await users in sequence {
+            if !started {
+                try await dataStore.delete(query: .all)
+            }
 
-    func fetchUsers() async throws -> [DisplayUser] {
-        let users = try await createFetchUsersTaskIfNeeded().value
-        self.users = users
-        return users
-    }
+            try await dataStore.store(users.compactMap { DisplayUser(user: $0) })
 
-    private func createFetchUsersTaskIfNeeded() -> Task<[DisplayUser], Error> {
-        if let fetchUsersTask {
-            return fetchUsersTask
-        }
-        let task = Task { [client] in
-            try await client
-                .api
-                .users
-                .listWithEditContext(params: UserListParams(perPage: 100))
-                .data
-                .compactMap { DisplayUser(user: $0) }
+            started = true
         }
-        fetchUsersTask = task
-        return task
     }
 
     func isCurrentUserCapableOf(_ capability: String) async -> Bool {
@@ -73,8 +52,8 @@ actor UserService: UserServiceProtocol {
         ).data
 
         // Remove the deleted user from the cached users list.
-        if result.deleted, let index = users?.firstIndex(where: { $0.id == id }) {
-            users?.remove(at: index)
+        if result.deleted {
+            try await dataStore.delete(query: .id([id]))
         }
     }
 

WordPress/Classes/Users/InMemoryUserDataStore.swift

@@ -0,0 +1,33 @@
+import Foundation
+import Combine
+
+public actor InMemoryUserDataStore: UserDataStore, InMemoryDataStore {
+    public typealias T = DisplayUser
+
+    public var storage: [T.ID: T] = [:]
+    public let updates: PassthroughSubject<Set<T.ID>, Never> = .init()
+
+    deinit {
+        updates.send(completion: .finished)
+    }
+
+    public func list(query: Query) throws -> [T] {
+        switch query {
+        case .all:
+            return Array(storage.values)
+        case let .id(ids):
+            return storage.reduce(into: []) {
+                if ids.contains($1.key) {
+                    $0.append($1.value)
+                }
+            }
+        case let .search(keyword):
+            let theKeyword = keyword.trimmingCharacters(in: .whitespacesAndNewlines)
+            if theKeyword.isEmpty {
+                return Array(storage.values)
+            } else {
+                return storage.values.search(theKeyword, using: \.searchString)
+            }
+        }
+    }
+}

WordPress/Classes/Users/UserProvider.swift

@@ -1,11 +1,19 @@
 import Foundation
 import Combine
 
+public protocol UserDataStore: DataStore where T == DisplayUser, Query == UserDataStoreQuery {
+}
+
+public enum UserDataStoreQuery: Equatable {
+    case all
+    case id(Set<DisplayUser.ID>)
+    case search(String)
+}
+
 public protocol UserServiceProtocol: Actor {
-    var users: [DisplayUser]? { get }
-    nonisolated var usersUpdates: AsyncStream<[DisplayUser]> { get }
+    var dataStore: any UserDataStore { get }
 
-    func fetchUsers() async throws -> [DisplayUser]
+    func fetchUsers() async throws
 
     func isCurrentUserCapableOf(_ capability: String) async -> Bool
 
@@ -24,6 +32,11 @@ actor MockUserProvider: UserServiceProtocol {
 
     var scenario: Scenario
 
+    private let _dataStore: InMemoryUserDataStore = .init()
+    var dataStore: any UserDataStore {
+        _dataStore
+    }
+
     nonisolated let usersUpdates: AsyncStream<[DisplayUser]>
     private let usersUpdatesContinuation: AsyncStream<[DisplayUser]>.Continuation
 
@@ -40,18 +53,17 @@ actor MockUserProvider: UserServiceProtocol {
         (usersUpdates, usersUpdatesContinuation) = AsyncStream<[DisplayUser]>.makeStream()
     }
 
-    func fetchUsers() async throws -> [DisplayUser] {
+    func fetchUsers() async throws {
         switch scenario {
         case .infinitLoading:
             // Do nothing
             try await Task.sleep(for: .seconds(24 * 60 * 60))
-            return []
         case .dummyData:
             let dummyDataUrl = URL(string: "https://my.api.mockaroo.com/users.json?key=067c9730")!
             let response = try await URLSession.shared.data(from: dummyDataUrl)
             let users = try JSONDecoder().decode([DisplayUser].self, from: response.0)
-            self.users = users
-            return users
+            try await _dataStore.delete(query: .all)
+            try await _dataStore.store(users)
         case .error:
             throw URLError(.timedOut)
         }

WordPress/Classes/Users/ViewModel/UserDeleteViewModel.swift

@@ -3,9 +3,6 @@ import SwiftUI
 @MainActor
 public class UserDeleteViewModel: ObservableObject {
 
-    @Published
-    private(set) var isFetchingOtherUsers: Bool = false
-
     @Published
     private(set) var isDeletingUser: Bool = false
 
@@ -16,40 +13,25 @@ public class UserDeleteViewModel: ObservableObject {
     var selectedUser: DisplayUser? = nil
 
     @Published
-    private(set) var otherUsers: [DisplayUser] = []
-
-    @Published
-    private(set) var deleteButtonIsDisabled: Bool = true
+    private(set) var otherUsers: [DisplayUser] = [] {
+        didSet {
+            if selectedUser == nil {
+                selectedUser = otherUsers.first
+            }
+        }
+    }
 
     private let userService: UserServiceProtocol
     let user: DisplayUser
 
     init(user: DisplayUser, userService: UserServiceProtocol) {
         self.user = user
         self.userService = userService
-
-        // Default `selectedUser` to be the first one in `otherUsers`.
-        // Using Combine here because `didSet` observers don't work with `@Published` properties.
-        //
-        // The implementation is equivalent to `if selectedUser == nil { selectedUser = otherUsers.first }`
-        $otherUsers.combineLatest($selectedUser)
-            .filter { _, selectedUser in selectedUser == nil }
-            .map { others, _ in others.first }
-            .assign(to: &$selectedUser)
-
     }
 
     func fetchOtherUsers() async {
-        isFetchingOtherUsers = true
-        deleteButtonIsDisabled = true
-
-        defer {
-            isFetchingOtherUsers = false
-            deleteButtonIsDisabled = otherUsers.isEmpty
-        }
-
         do {
-            let users = try await userService.fetchUsers()
+            let users = try await userService.dataStore.list(query: .all)
             self.otherUsers = users
                 .filter { $0.id != self.user.id } // Don't allow re-assigning to yourself
                 .sorted(using: KeyPathComparator(\.username))

WordPress/Classes/Users/ViewModel/UserListViewModel.swift

@@ -41,88 +41,72 @@ class UserListViewModel: ObservableObject {
         }
     }
 
-    /// The initial set of users fetched by `fetchItems`
-    private var users: [DisplayUser] = [] {
-        didSet {
-            sortedUsers = self.sortUsers(users)
-        }
-    }
-    private var updateUsersTask: Task<Void, Never>?
     private let userService: UserServiceProtocol
     private let currentUserId: Int32
     private var initialLoad = false
 
     @Published
-    private(set) var sortedUsers: [Section] = []
+    private(set) var query: UserDataStoreQuery = .all
 
     @Published
-    private(set) var error: Error? = nil
+    private(set) var sortedUsers: [Section] = []
 
     @Published
-    private(set) var isLoadingItems: Bool = true
+    private(set) var error: Error? = nil
 
     @Published
     var searchTerm: String = "" {
         didSet {
-            if searchTerm.trimmingCharacters(in: .whitespacesAndNewlines) == "" {
-                setSearchResults(sortUsers(users))
-            } else {
-                let searchResults = users.search(searchTerm, using: \.searchString)
-                setSearchResults([Section(id: .searchResult, users: searchResults)])
-            }
+            self.query = .search(searchTerm)
         }
     }
 
+    @Published
+    var isRefreshing: Bool = false
+    private var refreshItemsTask: Task<Void, Never>?
+
     init(userService: UserServiceProtocol, currentUserId: Int32) {
         self.userService = userService
         self.currentUserId = currentUserId
     }
 
     deinit {
-        updateUsersTask?.cancel()
+        refreshItemsTask?.cancel()
     }
 
     func onAppear() async {
-        if updateUsersTask == nil {
-            updateUsersTask = Task { @MainActor [weak self, usersUpdates = userService.usersUpdates] in
-                for await users in usersUpdates {
-                    guard let self else { break }
-
-                    self.users = users
-                }
-            }
-        }
-
         if !initialLoad {
             initialLoad = true
-            await fetchItems()
+            await refreshItems()
         }
     }
 
-    private func fetchItems() async {
-        isLoadingItems = true
-        defer { isLoadingItems = false }
-
-        _ = try? await userService.fetchUsers()
+    func performQuery() async {
+        let usersUpdates = await userService.dataStore.listStream(query: query)
+        for await users in usersUpdates {
+            switch users {
+            case let .success(users):
+                self.sortedUsers = self.sortUsers(users)
+            case let .failure(error):
+                self.error = error
+            }
+        }
     }
 
     @Sendable
     func refreshItems() async {
-        _ = try? await userService.fetchUsers()
-    }
-
-    func setUsers(_ newValue: [DisplayUser]) {
-        withAnimation {
-            self.users = newValue
-            self.sortedUsers = sortUsers(newValue)
-            isLoadingItems = false
+        refreshItemsTask?.cancel()
+        refreshItemsTask = Task {
+            isRefreshing = true
+            defer { isRefreshing = false }
+            do {
+                try await userService.fetchUsers()
+            } catch {
+                self.error = error
+            }
         }
-    }
 
-    func setSearchResults(_ newValue: [Section]) {
-        withAnimation {
-            self.sortedUsers = newValue
-        }
+        _ = await refreshItemsTask?.value
     }
 
     private func sortUsers(_ users: [DisplayUser]) -> [Section] {

WordPress/Classes/Users/Views/DeleteUserConfirmationSheet.swift

@@ -16,15 +16,9 @@ struct DeleteUserConfirmationSheet: View {
         NavigationView {
             Form {
                 Section {
-                    if deleteUserViewModel.isFetchingOtherUsers {
-                        LabeledContent(Strings.attributeContentToUserLabel) {
-                            ProgressView()
-                        }
-                    } else {
-                        Picker(Strings.attributeContentToUserLabel, selection: $deleteUserViewModel.selectedUser) {
-                            ForEach(deleteUserViewModel.otherUsers) { user in
-                                Text("\(user.displayName) (\(user.username))").tag(user)
-                            }
+                    Picker(Strings.attributeContentToUserLabel, selection: $deleteUserViewModel.selectedUser) {
+                        ForEach(deleteUserViewModel.otherUsers) { user in
+                            Text("\(user.displayName) (\(user.username))").tag(user)
                         }
                     }
                 } header: {
@@ -53,7 +47,6 @@ struct DeleteUserConfirmationSheet: View {
                     } label: {
                         Text(Strings.attributeContentConfirmationDeleteButton)
                     }
-                    .disabled(deleteUserViewModel.deleteButtonIsDisabled)
                 }
             }
             .onAppear {