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 75f15dc9b4aafcd6dbd741651a2e18d8f1fe3f7e Mon Sep 17 00:00:00 2001 From: Tony Li Date: Fri, 29 Nov 2024 23:22:42 +1300 Subject: [PATCH 3/3] Create DataStoreQuery type to perform query --- WordPress/Classes/Services/UserService.swift | 6 +- .../Classes/Users/InMemoryUserDataStore.swift | 31 ---------- WordPress/Classes/Users/UserProvider.swift | 17 ++---- .../Classes/Utility/DataStore/DataStore.swift | 56 +++++++++++++++++-- .../Utility/DataStore/InMemoryDataStore.swift | 24 +++++--- WordPress/WordPressTest/DataStoreTests.swift | 8 +-- 6 files changed, 76 insertions(+), 66 deletions(-) diff --git a/WordPress/Classes/Services/UserService.swift b/WordPress/Classes/Services/UserService.swift index 5a9b2dbea872..f37d7a0a0b3b 100644 --- a/WordPress/Classes/Services/UserService.swift +++ b/WordPress/Classes/Services/UserService.swift @@ -8,8 +8,8 @@ import WordPressUI actor UserService: UserServiceProtocol, UserDataStoreProvider { private let client: WordPressClient - private let _dataStore: InMemoryUserDataStore = .init() - var userDataStore: any UserDataStore { _dataStore } + private let _dataStore: InMemoryDataStore = .init() + var userDataStore: any DataStore { _dataStore } private var _currentUser: UserWithEditContext? private var currentUser: UserWithEditContext? { @@ -51,7 +51,7 @@ actor UserService: UserServiceProtocol, UserDataStoreProvider { // Remove the deleted user from the cached users list. if result.deleted { - try await _dataStore.delete(query: .id([id])) + try await _dataStore.delete(query: .identifier(in: [id])) } } diff --git a/WordPress/Classes/Users/InMemoryUserDataStore.swift b/WordPress/Classes/Users/InMemoryUserDataStore.swift index 3f60f41e1023..9206577bf681 100644 --- a/WordPress/Classes/Users/InMemoryUserDataStore.swift +++ b/WordPress/Classes/Users/InMemoryUserDataStore.swift @@ -1,33 +1,2 @@ import Foundation import Combine - -public actor InMemoryUserDataStore: UserDataStore, InMemoryDataStore { - public typealias T = DisplayUser - - public var storage: [T.ID: T] = [:] - public let updates: PassthroughSubject, 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) - } - } - } -} diff --git a/WordPress/Classes/Users/UserProvider.swift b/WordPress/Classes/Users/UserProvider.swift index 4afeee6957dc..0fada2f54298 100644 --- a/WordPress/Classes/Users/UserProvider.swift +++ b/WordPress/Classes/Users/UserProvider.swift @@ -1,15 +1,6 @@ import Foundation import Combine -public protocol UserDataStore: DataStore where T == DisplayUser, Query == UserDataStoreQuery { -} - -public enum UserDataStoreQuery: Equatable { - case all - case id(Set) - case search(String) -} - public protocol UserServiceProtocol: Actor { func fetchUsers() async throws @@ -27,7 +18,7 @@ public protocol UserServiceProtocol: Actor { } protocol UserDataStoreProvider: Actor { - var userDataStore: any UserDataStore { get } + var userDataStore: any DataStore { get } } extension UserServiceProtocol where Self: UserDataStoreProvider { @@ -36,7 +27,7 @@ extension UserServiceProtocol where Self: UserDataStoreProvider { } func streamSearchResult(input: String) async -> AsyncStream> { - await userDataStore.listStream(query: .search(input)) + await userDataStore.listStream(query: .search(input, transform: \.searchString)) } func streamAll() async -> AsyncStream> { @@ -54,8 +45,8 @@ actor MockUserProvider: UserServiceProtocol, UserDataStoreProvider { var scenario: Scenario - private let _dataStore: InMemoryUserDataStore = .init() - var userDataStore: any UserDataStore { _dataStore } + private let _dataStore: InMemoryDataStore = .init() + var userDataStore: any DataStore { _dataStore } nonisolated let usersUpdates: AsyncStream<[DisplayUser]> private let usersUpdatesContinuation: AsyncStream<[DisplayUser]>.Continuation diff --git a/WordPress/Classes/Utility/DataStore/DataStore.swift b/WordPress/Classes/Utility/DataStore/DataStore.swift index a20823f1c529..2f46aa669ffc 100644 --- a/WordPress/Classes/Utility/DataStore/DataStore.swift +++ b/WordPress/Classes/Utility/DataStore/DataStore.swift @@ -1,16 +1,60 @@ import Foundation +import WordPressShared /// An abstraction of local data storage, with CRUD operations. -public protocol DataStore: Actor { - associatedtype T: Identifiable & Sendable - associatedtype Query +public protocol DataStore: Actor { + associatedtype T: Identifiable & Sendable where T.ID: Sendable - func list(query: Query) async throws -> [T] - func delete(query: Query) async throws + func list(query: DataStoreQuery) async throws -> [T] + func delete(query: DataStoreQuery) async throws func store(_ data: [T]) async throws /// An AsyncStream that produces up-to-date results for the given query. /// /// The `AsyncStream` should not finish as long as the `DataStore` remains alive and valid. - func listStream(query: Query) -> AsyncStream> + func listStream(query: DataStoreQuery) -> AsyncStream> +} + +public struct DataStoreQuery: Sendable where T.ID: Sendable { + public indirect enum Filter: Sendable { + case identifier(Set) + case closure(@Sendable (T) -> Bool) + case and(lhs: Filter, rhs: Filter) + case or(lhs: Filter, rhs: Filter) + + func evaluate(on value: T) -> Bool { + switch self { + case let .identifier(ids): + ids.contains(value.id) + case let .closure(closure): + closure(value) + case let .and(lhs, rhs): + lhs.evaluate(on: value) && rhs.evaluate(on: value) + case let .or(lhs, rhs): + lhs.evaluate(on: value) || rhs.evaluate(on: value) + } + } + } + + var filter: Filter? + var sortBy: [SortDescriptor] = [] + + public func perform(on data: any Sequence) -> [T] { + var result: any Sequence = data + if let filter { + result = result.filter { filter.evaluate(on: $0) } + } + return result.sorted(using: sortBy) + } + + public static var all: Self { .init() } + + public static func identifier(in ids: Set) -> Self { + .init(filter: .identifier(ids)) + } + + public static func search(_ query: String, minScore: Double = 0.7, transform: @escaping (T) -> String) -> Self { + let term = StringRankedSearch(searchTerm: query) + return .init(filter: .closure { term.score(for: transform($0)) >= minScore }) + } } diff --git a/WordPress/Classes/Utility/DataStore/InMemoryDataStore.swift b/WordPress/Classes/Utility/DataStore/InMemoryDataStore.swift index 0ad8071ca0f1..b10a88461777 100644 --- a/WordPress/Classes/Utility/DataStore/InMemoryDataStore.swift +++ b/WordPress/Classes/Utility/DataStore/InMemoryDataStore.swift @@ -2,22 +2,24 @@ import Foundation import Combine /// A `DataStore` type that stores data in memory. -public protocol InMemoryDataStore: DataStore { +public actor InMemoryDataStore: DataStore where T.ID: Sendable { /// A `Dictionary` to store the data in memory. - var storage: [T.ID: T] { get set } + var storage: [T.ID: T] = [:] /// A publisher for sending and subscribing data changes. /// /// The publisher emits events when data changes, with identifiers of changed models. /// /// The publisher does not complete as long as the `InMemoryDataStore` remains alive and valid. - var updates: PassthroughSubject, Never> { get } -} + let updates: PassthroughSubject, Never> = .init() + + deinit { + updates.send(completion: .finished) + } -public extension InMemoryDataStore { - func delete(query: Query) async throws { + public func delete(query: DataStoreQuery) async throws { var updated = Set() - let result = try await list(query: query) + let result = try list(query: query) result.forEach { if storage.removeValue(forKey: $0.id) != nil { updated.insert($0.id) @@ -29,7 +31,7 @@ public extension InMemoryDataStore { } } - func store(_ data: [T]) async throws { + public func store(_ data: [T]) async throws { var updated = Set() data.forEach { updated.insert($0.id) @@ -41,7 +43,11 @@ public extension InMemoryDataStore { } } - func listStream(query: Query) -> AsyncStream> { + public func list(query: DataStoreQuery) throws -> [T] { + query.perform(on: storage.values) + } + + public func listStream(query: DataStoreQuery) -> AsyncStream> { let stream = AsyncStream>.makeStream() let updatingTask = Task { [weak self] in diff --git a/WordPress/WordPressTest/DataStoreTests.swift b/WordPress/WordPressTest/DataStoreTests.swift index 3305a61b5b37..3caa98bb6c37 100644 --- a/WordPress/WordPressTest/DataStoreTests.swift +++ b/WordPress/WordPressTest/DataStoreTests.swift @@ -8,7 +8,7 @@ struct InMemoryDataStoreTests { @Test func testUpdatesAfterCreation() async { - let store: InMemoryUserDataStore = InMemoryUserDataStore() + let store = InMemoryDataStore() let stream = await store.listStream(query: .all) await confirmation("The stream produces an update") { confirmation in @@ -20,7 +20,7 @@ struct InMemoryDataStoreTests { @Test func testUpdatesAfterStore() async { - let store: InMemoryUserDataStore = InMemoryUserDataStore() + let store = InMemoryDataStore() let stream = await store.listStream(query: .all) Task.detached { @@ -37,7 +37,7 @@ struct InMemoryDataStoreTests { @Test func testUpdatesAfterDelete() async throws { - let store: InMemoryUserDataStore = InMemoryUserDataStore() + let store = InMemoryDataStore() try await store.store([.MockUser]) let stream = await store.listStream(query: .all) @@ -56,7 +56,7 @@ struct InMemoryDataStoreTests { @Test func testStreamTerminates() async { - var store: InMemoryUserDataStore? = InMemoryUserDataStore() + var store: InMemoryDataStore? = .init() let stream = await store!.listStream(query: .all) Task.detached {