From ecb2b33b17e5b0358f91e87822d3f16641d1883a Mon Sep 17 00:00:00 2001 From: Tony Li Date: Fri, 29 Nov 2024 14:28:22 +1300 Subject: [PATCH 1/3] Encapsulate UserDataStore in UserService --- WordPress/Classes/Services/UserService.swift | 12 +++---- WordPress/Classes/Users/UserProvider.swift | 36 ++++++++++++++----- .../Users/ViewModel/UserDeleteViewModel.swift | 3 +- .../Users/ViewModel/UserListViewModel.swift | 18 ++++++++-- .../Classes/Users/Views/UserListView.swift | 2 +- .../UserListViewModelTests.swift | 28 +++++++++++---- 6 files changed, 71 insertions(+), 28 deletions(-) diff --git a/WordPress/Classes/Services/UserService.swift b/WordPress/Classes/Services/UserService.swift index 21417ac17b8d..5a9b2dbea872 100644 --- a/WordPress/Classes/Services/UserService.swift +++ b/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])) } } diff --git a/WordPress/Classes/Users/UserProvider.swift b/WordPress/Classes/Users/UserProvider.swift index a835d732d083..4afeee6957dc 100644 --- a/WordPress/Classes/Users/UserProvider.swift +++ b/WordPress/Classes/Users/UserProvider.swift @@ -11,8 +11,6 @@ public enum UserDataStoreQuery: Equatable { } public protocol UserServiceProtocol: Actor { - var dataStore: any UserDataStore { get } - func fetchUsers() async throws func isCurrentUserCapableOf(_ capability: String) async -> Bool @@ -20,9 +18,33 @@ public protocol UserServiceProtocol: Actor { 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> + + func streamAll() async -> AsyncStream> +} + +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> { + await userDataStore.listStream(query: .search(input)) + } + + func streamAll() async -> AsyncStream> { + 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) } diff --git a/WordPress/Classes/Users/ViewModel/UserDeleteViewModel.swift b/WordPress/Classes/Users/ViewModel/UserDeleteViewModel.swift index de5a05b23a43..78866f1ce64a 100644 --- a/WordPress/Classes/Users/ViewModel/UserDeleteViewModel.swift +++ b/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 { diff --git a/WordPress/Classes/Users/ViewModel/UserListViewModel.swift b/WordPress/Classes/Users/ViewModel/UserListViewModel.swift index 7c0b4d16e21b..e361f6598bda 100644 --- a/WordPress/Classes/Users/ViewModel/UserListViewModel.swift +++ b/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,7 @@ class UserListViewModel: ObservableObject { @Published var searchTerm: String = "" { didSet { - self.query = .search(searchTerm) + self.mode = .search(searchTerm) } } @@ -82,7 +87,14 @@ class UserListViewModel: ObservableObject { } func performQuery() async { - let usersUpdates = await userService.dataStore.listStream(query: query) + let usersUpdates: AsyncStream> + 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): diff --git a/WordPress/Classes/Users/Views/UserListView.swift b/WordPress/Classes/Users/Views/UserListView.swift index 1fdf7bf49526..c683173830a9 100644 --- a/WordPress/Classes/Users/Views/UserListView.swift +++ b/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() } diff --git a/WordPress/WordPressTest/UserListViewModelTests.swift b/WordPress/WordPressTest/UserListViewModelTests.swift index 1b9d81ea75b4..356292b85732 100644 --- a/WordPress/WordPressTest/UserListViewModelTests.swift +++ b/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() { From 80797bc26a9867d2cb4891b645bae04c0109824f Mon Sep 17 00:00:00 2001 From: Tony Li Date: Fri, 29 Nov 2024 23:23:50 +1300 Subject: [PATCH 2/3] Display all users when search keyword is empty --- WordPress/Classes/Users/ViewModel/UserListViewModel.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/WordPress/Classes/Users/ViewModel/UserListViewModel.swift b/WordPress/Classes/Users/ViewModel/UserListViewModel.swift index e361f6598bda..3d780976434f 100644 --- a/WordPress/Classes/Users/ViewModel/UserListViewModel.swift +++ b/WordPress/Classes/Users/ViewModel/UserListViewModel.swift @@ -62,7 +62,8 @@ class UserListViewModel: ObservableObject { @Published var searchTerm: String = "" { didSet { - self.mode = .search(searchTerm) + let keyword = searchTerm.trimmingCharacters(in: .whitespacesAndNewlines) + self.mode = keyword.isEmpty ? .allUsers : .search(keyword) } } From e5858bd97c22cdedab78caa839acea6833a59a16 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Fri, 29 Nov 2024 23:36:07 +1300 Subject: [PATCH 3/3] Fix a swiftlint issue --- WordPress/WordPressTest/UserListViewModelTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/WordPressTest/UserListViewModelTests.swift b/WordPress/WordPressTest/UserListViewModelTests.swift index 356292b85732..8c76eac16443 100644 --- a/WordPress/WordPressTest/UserListViewModelTests.swift +++ b/WordPress/WordPressTest/UserListViewModelTests.swift @@ -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.streamAll(){ + for await _ in await self.service.streamAll() { expectation.fulfill() } }