Encapsulate UserDataStore in UserService (#23866)

* Encapsulate UserDataStore in UserService

* Display all users when search keyword is empty

* Fix a swiftlint issue

WordPress/Classes/Services/UserService.swift

@@ -5,13 +5,11 @@ import WordPressUI
 
 /// UserService is responsible for fetching user acounts via the .org REST API – it's the replacement for `UsersService` (the XMLRPC-based approach)
 ///
-actor UserService: UserServiceProtocol {
+actor UserService: UserServiceProtocol, UserDataStoreProvider {
     private let client: WordPressClient
 
     private let _dataStore: InMemoryUserDataStore = .init()
-    var dataStore: any UserDataStore {
-        _dataStore
-    }
+    var userDataStore: any UserDataStore { _dataStore }
 
     private var _currentUser: UserWithEditContext?
     private var currentUser: UserWithEditContext? {
@@ -32,10 +30,10 @@ actor UserService: UserServiceProtocol {
         var started = false
         for try await users in sequence {
             if !started {
-                try await dataStore.delete(query: .all)
+                try await _dataStore.delete(query: .all)
             }
 
-            try await dataStore.store(users.compactMap { DisplayUser(user: $0) })
+            try await _dataStore.store(users.compactMap { DisplayUser(user: $0) })
 
             started = true
         }
@@ -53,7 +51,7 @@ actor UserService: UserServiceProtocol {
 
         // Remove the deleted user from the cached users list.
         if result.deleted {
-            try await dataStore.delete(query: .id([id]))
+            try await _dataStore.delete(query: .id([id]))
         }
     }
 

WordPress/Classes/Users/UserProvider.swift

@@ -11,18 +11,40 @@ public enum UserDataStoreQuery: Equatable {
 }
 
 public protocol UserServiceProtocol: Actor {
-    var dataStore: any UserDataStore { get }
-
     func fetchUsers() async throws
 
     func isCurrentUserCapableOf(_ capability: String) async -> Bool
 
     func setNewPassword(id: Int32, newPassword: String) async throws
 
     func deleteUser(id: Int32, reassigningPostsTo newUserId: Int32) async throws
+
+    func allUsers() async throws -> [DisplayUser]
+
+    func streamSearchResult(input: String) async -> AsyncStream<Result<[DisplayUser], Error>>
+
+    func streamAll() async -> AsyncStream<Result<[DisplayUser], Error>>
+}
+
+protocol UserDataStoreProvider: Actor {
+    var userDataStore: any UserDataStore { get }
+}
+
+extension UserServiceProtocol where Self: UserDataStoreProvider {
+    func allUsers() async throws -> [DisplayUser] {
+        try await userDataStore.list(query: .all)
+    }
+
+    func streamSearchResult(input: String) async -> AsyncStream<Result<[DisplayUser], Error>> {
+        await userDataStore.listStream(query: .search(input))
+    }
+
+    func streamAll() async -> AsyncStream<Result<[DisplayUser], Error>> {
+        await userDataStore.listStream(query: .all)
+    }
 }
 
-actor MockUserProvider: UserServiceProtocol {
+actor MockUserProvider: UserServiceProtocol, UserDataStoreProvider {
 
     enum Scenario {
         case infinitLoading
@@ -33,9 +55,7 @@ actor MockUserProvider: UserServiceProtocol {
     var scenario: Scenario
 
     private let _dataStore: InMemoryUserDataStore = .init()
-    var dataStore: any UserDataStore {
-        _dataStore
-    }
+    var userDataStore: any UserDataStore { _dataStore }
 
     nonisolated let usersUpdates: AsyncStream<[DisplayUser]>
     private let usersUpdatesContinuation: AsyncStream<[DisplayUser]>.Continuation
@@ -62,8 +82,8 @@ actor MockUserProvider: UserServiceProtocol {
             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)
-            try await _dataStore.delete(query: .all)
-            try await _dataStore.store(users)
+            try await userDataStore.delete(query: .all)
+            try await userDataStore.store(users)
         case .error:
             throw URLError(.timedOut)
         }

WordPress/Classes/Users/ViewModel/UserDeleteViewModel.swift

@@ -31,8 +31,7 @@ public class UserDeleteViewModel: ObservableObject {
 
     func fetchOtherUsers() async {
         do {
-            let users = try await userService.dataStore.list(query: .all)
-            self.otherUsers = users
+            self.otherUsers = try await userService.allUsers()
                 .filter { $0.id != self.user.id } // Don't allow re-assigning to yourself
                 .sorted(using: KeyPathComparator(\.username))
         } catch {

WordPress/Classes/Users/ViewModel/UserListViewModel.swift

@@ -5,6 +5,11 @@ import WordPressShared
 @MainActor
 class UserListViewModel: ObservableObject {
 
+    enum Mode: Equatable {
+        case allUsers
+        case search(String)
+    }
+
     enum RoleSection: Hashable, Comparable {
         case me
         case role(String)
@@ -46,7 +51,7 @@ class UserListViewModel: ObservableObject {
     private var initialLoad = false
 
     @Published
-    private(set) var query: UserDataStoreQuery = .all
+    private(set) var mode: Mode = .allUsers
 
     @Published
     private(set) var sortedUsers: [Section] = []
@@ -57,7 +62,8 @@ class UserListViewModel: ObservableObject {
     @Published
     var searchTerm: String = "" {
         didSet {
-            self.query = .search(searchTerm)
+            let keyword = searchTerm.trimmingCharacters(in: .whitespacesAndNewlines)
+            self.mode = keyword.isEmpty ? .allUsers : .search(keyword)
         }
     }
 
@@ -82,7 +88,14 @@ class UserListViewModel: ObservableObject {
     }
 
     func performQuery() async {
-        let usersUpdates = await userService.dataStore.listStream(query: query)
+        let usersUpdates: AsyncStream<Result<[DisplayUser], Error>>
+        switch mode {
+        case .allUsers:
+            usersUpdates = await userService.streamAll()
+        case let .search(keyword):
+            usersUpdates = await userService.streamSearchResult(input: keyword)
+        }
+
         for await users in usersUpdates {
             switch users {
             case let .success(users):

WordPress/Classes/Users/Views/UserListView.swift

@@ -58,7 +58,7 @@ public struct UserListView: View {
                 }
             }
         }
-        .task(id: viewModel.query) {
+        .task(id: viewModel.mode) {
             await viewModel.performQuery()
         }
         .task { await viewModel.onAppear() }

WordPress/WordPressTest/UserListViewModelTests.swift

@@ -30,7 +30,7 @@ class UserListViewModelTests: XCTestCase {
 
         let expectation = XCTestExpectation(description: "Updated after fetch")
         let task = Task.detached {
-            for await _ in await self.service.dataStore.listStream(query: .all) {
+            for await _ in await self.service.streamAll() {
                 expectation.fulfill()
             }
         }
@@ -53,7 +53,7 @@ class UserListViewModelTests: XCTestCase {
         let expectation = XCTestExpectation(description: "Updated after fetch")
         expectation.expectedFulfillmentCount = 5
         let task = Task.detached {
-            for await _ in await self.service.dataStore.listStream(query: .all) {
+            for await _ in await self.service.streamAll() {
                 expectation.fulfill()
             }
         }
@@ -72,7 +72,7 @@ class UserListViewModelTests: XCTestCase {
 
         let termination = XCTestExpectation(description: "Stream has finished")
         let task = Task.detached { [self] in
-            for await _ in await self.service.dataStore.listStream(query: .all) {
+            for await _ in await self.service.streamAll() {
                 // Do nothing
             }
             termination.fulfill()
@@ -96,12 +96,26 @@ class UserListViewModelTests: XCTestCase {
         stubDeleteUser(id: 34)
 
         _ = await viewModel.refreshItems()
-        let userFetched = try await service.dataStore.list(query: .id([34])).isEmpty == false
-        XCTAssertTrue(userFetched)
+
+        let userExisted = expectation(description: "User 34 exists")
+        let userDeleted = expectation(description: "User 34 is deleted")
+
+        let subscription = Task.detached {
+            for await result in await self.service.streamAll() {
+                let users = try result.get()
+                if users.contains(where: { $0.id == 34 }) {
+                    userExisted.fulfill()
+                } else {
+                    userDeleted.fulfill()
+                }
+            }
+        }
 
         try await service.deleteUser(id: 34, reassigningPostsTo: 1)
-        let userDeleted = try await service.dataStore.list(query: .id([34])).isEmpty
-        XCTAssertTrue(userDeleted)
+
+        await fulfillment(of: [userExisted, userDeleted], timeout: 0.3, enforceOrder: true)
+
+        subscription.cancel()
     }
 
     private func stubSuccessfullUsersFetch() {