diff --git a/Modules/Package.swift b/Modules/Package.swift index 4b5494e85591..b18d5804e29e 100644 --- a/Modules/Package.swift +++ b/Modules/Package.swift @@ -46,7 +46,7 @@ let package = Package( .package(url: "https://github.com/wordpress-mobile/WordPressKit-iOS", branch: "task/reader-discover"), .package(url: "https://github.com/zendesk/support_sdk_ios", from: "8.0.3"), // We can't use wordpress-rs branches nor commits here. Only tags work. - .package(url: "https://github.com/Automattic/wordpress-rs", revision: "alpha-swift-20240813"), + .package(url: "https://github.com/Automattic/wordpress-rs", revision: "alpha-20241116"), .package(url: "https://github.com/wordpress-mobile/GutenbergKit", revision: "6cc307e7fc24910697be5f71b7d70f465a9c0f63"), .package(url: "https://github.com/Automattic/color-studio", branch: "trunk"), ], @@ -161,7 +161,7 @@ enum XcodeSupport { .product(name: "ZendeskSupportSDK", package: "support_sdk_ios"), .product(name: "ZIPFoundation", package: "ZIPFoundation"), .product(name: "WordPressAPI", package: "wordpress-rs"), - .product(name: "ColorStudio", package: "color-studio"), + .product(name: "ColorStudio", package: "color-studio") ]), .xcodeTarget("XcodeTarget_WordPressTests", dependencies: testDependencies + [ "WordPressShared", diff --git a/Modules/Sources/WordPressUI/Views/Users/Components/UserListItem.swift b/Modules/Sources/WordPressUI/Views/Users/Components/UserListItem.swift index 93c21253aa7a..cd828d6e8a1f 100644 --- a/Modules/Sources/WordPressUI/Views/Users/Components/UserListItem.swift +++ b/Modules/Sources/WordPressUI/Views/Users/Components/UserListItem.swift @@ -32,7 +32,7 @@ struct UserListItem: View { } } } - -#Preview { - UserListItem(user: DisplayUser.MockUser, userService: MockUserProvider()) -} +// +//#Preview { +// UserListItem(user: DisplayUser.MockUser, userService: MockUserProvider()) +//} diff --git a/Modules/Sources/WordPressUI/Views/Users/UserProvider.swift b/Modules/Sources/WordPressUI/Views/Users/UserProvider.swift index 59b1cda47221..8222a8f55a02 100644 --- a/Modules/Sources/WordPressUI/Views/Users/UserProvider.swift +++ b/Modules/Sources/WordPressUI/Views/Users/UserProvider.swift @@ -2,10 +2,7 @@ import Foundation import Combine public protocol UserServiceProtocol: Actor { - var users: [DisplayUser]? { get } - nonisolated var usersUpdates: AsyncStream<[DisplayUser]> { get } - - func fetchUsers() async throws -> [DisplayUser] + func fetchPaginatedUsers() -> AsyncThrowingStream<[DisplayUser], Error> func isCurrentUserCapableOf(_ capability: String) async throws -> Bool @@ -13,59 +10,51 @@ public protocol UserServiceProtocol: Actor { func deleteUser(id: Int32, reassigningPostsTo newUserId: Int32) async throws } - -package actor MockUserProvider: UserServiceProtocol { - - enum Scenario { - case infinitLoading - case dummyData - case error - } - - var scenario: Scenario - - package nonisolated let usersUpdates: AsyncStream<[DisplayUser]> - private let usersUpdatesContinuation: AsyncStream<[DisplayUser]>.Continuation - - package private(set) var users: [DisplayUser]? { - didSet { - if let users { - usersUpdatesContinuation.yield(users) - } - } - } - - init(scenario: Scenario = .dummyData) { - self.scenario = scenario - (usersUpdates, usersUpdatesContinuation) = AsyncStream<[DisplayUser]>.makeStream() - } - - package func fetchUsers() async throws -> [DisplayUser] { - 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 - case .error: - throw URLError(.timedOut) - } - } - - package func isCurrentUserCapableOf(_ capability: String) async throws -> Bool { - true - } - - package func setNewPassword(id: Int32, newPassword: String) async throws { - // Not used in Preview - } - - package func deleteUser(id: Int32, reassigningPostsTo newUserId: Int32) async throws { - // Not used in Preview - } -} +// +//package actor MockUserProvider: UserServiceProtocol { +// +// enum Scenario { +// case infinitLoading +// case dummyData +// case error +// } +// +// var scenario: Scenario +// +// init(scenario: Scenario = .dummyData) { +// self.scenario = scenario +// } +// +// public func fetchUsers() async throws -> any AsyncSequence { +// [DisplayUser]().async +// } +// +// +// package func fetchUsers() async throws -> [DisplayUser] { +// 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) +// return users +// case .error: +// throw URLError(.timedOut) +// } +// } +// +// package func isCurrentUserCapableOf(_ capability: String) async throws -> Bool { +// true +// } +// +// package func setNewPassword(id: Int32, newPassword: String) async throws { +// // Not used in Preview +// } +// +// package func deleteUser(id: Int32, reassigningPostsTo newUserId: Int32) async throws { +// // Not used in Preview +// } +//} diff --git a/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDeleteViewModel.swift b/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDeleteViewModel.swift index 6736f1e8365c..c33c83f92715 100644 --- a/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDeleteViewModel.swift +++ b/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserDeleteViewModel.swift @@ -49,8 +49,11 @@ public class UserDeleteViewModel: ObservableObject { } do { - let users = try await userService.fetchUsers() - self.otherUsers = users + let users = await userService.fetchPaginatedUsers() + self.otherUsers = try await users + .reduce(into: [DisplayUser](), { partialResult, users in + partialResult.append(contentsOf: users) + }) .filter { $0.id != self.user.id } // Don't allow re-assigning to yourself .sorted(using: KeyPathComparator(\.username)) } catch { diff --git a/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserListViewModel.swift b/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserListViewModel.swift index 8ecab8e027d3..fe3bf9858758 100644 --- a/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserListViewModel.swift +++ b/Modules/Sources/WordPressUI/Views/Users/ViewModel/UserListViewModel.swift @@ -12,9 +12,9 @@ class UserListViewModel: ObservableObject { } /// The initial set of users fetched by `fetchItems` - private var users: [DisplayUser] = [] { + private var users: [Int32: DisplayUser] = [:] { didSet { - sortedUsers = self.sortUsers(users) + sortedUsers = self.sortUsers(Array(users.values)) } } private var updateUsersTask: Task<Void, Never>? @@ -36,7 +36,7 @@ class UserListViewModel: ObservableObject { if searchTerm.trimmingCharacters(in: .whitespacesAndNewlines) == "" { setSearchResults(sortUsers(users)) } else { - let searchResults = users.search(searchTerm, using: \.searchString) + let searchResults = users.values.search(searchTerm, using: \.searchString) setSearchResults([Section(role: "Search Results", users: searchResults)]) } } @@ -51,16 +51,6 @@ class UserListViewModel: ObservableObject { } 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() @@ -68,24 +58,33 @@ class UserListViewModel: ObservableObject { } private func fetchItems() async { - isLoadingItems = true - defer { isLoadingItems = false } - _ = try? await userService.fetchUsers() + do { + for try await page in await userService.fetchPaginatedUsers() { + for user in page { + self.users[user.id] = user + } + + // Show results after the first page has loaded + isLoadingItems = false + } + } catch { + self.error = error + } } @Sendable func refreshItems() async { - _ = try? await userService.fetchUsers() + await fetchItems() } - func setUsers(_ newValue: [DisplayUser]) { - withAnimation { - self.users = newValue - self.sortedUsers = sortUsers(newValue) - isLoadingItems = false - } - } +// func setUsers(_ newValue: [DisplayUser]) { +// withAnimation { +// self.users = newValue +// self.sortedUsers = sortUsers(newValue) +// isLoadingItems = false +// } +// } func setSearchResults(_ newValue: [Section]) { withAnimation { @@ -98,4 +97,10 @@ class UserListViewModel: ObservableObject { .map { Section(role: $0.key, users: $0.value.sorted(by: { $0.username < $1.username })) } .sorted { $0.role < $1.role } } + + private func sortUsers(_ users: [Int32: DisplayUser]) -> [Section] { + Dictionary(grouping: users.values, by: { $0.role }) + .map { Section(role: $0.key, users: $0.value.sorted(by: { $0.username < $1.username })) } + .sorted { $0.role < $1.role } + } } diff --git a/Modules/Sources/WordPressUI/Views/Users/Views/UserDetailsView.swift b/Modules/Sources/WordPressUI/Views/Users/Views/UserDetailsView.swift index 39d8e0457f47..d8a1cfeee78c 100644 --- a/Modules/Sources/WordPressUI/Views/Users/Views/UserDetailsView.swift +++ b/Modules/Sources/WordPressUI/Views/Users/Views/UserDetailsView.swift @@ -380,8 +380,8 @@ private extension String { } } -#Preview { - NavigationStack { - UserDetailsView(user: DisplayUser.MockUser, userService: MockUserProvider()) - } -} +//#Preview { +// NavigationStack { +// UserDetailsView(user: DisplayUser.MockUser, userService: MockUserProvider()) +// } +//} diff --git a/Modules/Sources/WordPressUI/Views/Users/Views/UserListView.swift b/Modules/Sources/WordPressUI/Views/Users/Views/UserListView.swift index 31cb09e7775a..d1e2cc909673 100644 --- a/Modules/Sources/WordPressUI/Views/Users/Views/UserListView.swift +++ b/Modules/Sources/WordPressUI/Views/Users/Views/UserListView.swift @@ -67,21 +67,21 @@ public struct UserListView: View { ) } } - -#Preview("Loading") { - NavigationView { - UserListView(userService: MockUserProvider()) - } -} - -#Preview("Error") { - NavigationView { - UserListView(userService: MockUserProvider(scenario: .error)) - } -} - -#Preview("List") { - NavigationView { - UserListView(userService: MockUserProvider(scenario: .dummyData)) - } -} +// +//#Preview("Loading") { +// NavigationView { +// UserListView(userService: MockUserProvider()) +// } +//} +// +//#Preview("Error") { +// NavigationView { +// UserListView(userService: MockUserProvider(scenario: .error)) +// } +//} +// +//#Preview("List") { +// NavigationView { +// UserListView(userService: MockUserProvider(scenario: .dummyData)) +// } +//} diff --git a/WordPress.xcworkspace/xcshareddata/swiftpm/Package.resolved b/WordPress.xcworkspace/xcshareddata/swiftpm/Package.resolved index 6e5903f96a77..40c6ff3f2a52 100644 --- a/WordPress.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/WordPress.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -372,8 +372,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/Automattic/wordpress-rs", "state" : { - "branch" : "alpha-swift-20240813", - "revision" : "b51c560d83a61917eb3361441e65779b71fb96ce" + "branch" : "alpha-20241116", + "revision" : "1249ae77fcea2e836b7878a1b1cffdf6c1080256" } }, { diff --git a/WordPress/Classes/Networking/WordPressClient.swift b/WordPress/Classes/Networking/WordPressClient.swift index e3176f6df152..03d3c8c4c6f3 100644 --- a/WordPress/Classes/Networking/WordPressClient.swift +++ b/WordPress/Classes/Networking/WordPressClient.swift @@ -78,7 +78,7 @@ actor WordPressClient { try await self.api.plugins.create(params: PluginCreateParams( slug: "InstallJetpack", status: .active - )) + )).data } } diff --git a/WordPress/Classes/Services/ApplicationPasswordService.swift b/WordPress/Classes/Services/ApplicationPasswordService.swift index fb4cb414865f..7289367b2fb7 100644 --- a/WordPress/Classes/Services/ApplicationPasswordService.swift +++ b/WordPress/Classes/Services/ApplicationPasswordService.swift @@ -12,7 +12,7 @@ import WordPressAPI } private func fetchTokens(forUserId userId: Int32) async throws -> [ApplicationPasswordWithEditContext] { - try await apiClient.api.applicationPasswords.listWithEditContext(userId: userId) + try await apiClient.api.applicationPasswords.listWithEditContext(userId: userId).data } } diff --git a/WordPress/Classes/Services/UserService.swift b/WordPress/Classes/Services/UserService.swift index f684dc20b93f..1affd46b6fdb 100644 --- a/WordPress/Classes/Services/UserService.swift +++ b/WordPress/Classes/Services/UserService.swift @@ -3,54 +3,86 @@ import Combine import WordPressAPI import WordPressUI +protocol ObjectCache<Cacheable>: Actor { + + associatedtype Cacheable: (Identifiable & Sendable) + + func get(_ id: Cacheable.ID) -> Cacheable? + func store(_ object: Cacheable) + func store(_ objects: [Cacheable]) + func all() -> [Cacheable] + func invalidate(_ id: Cacheable.ID) +} + +actor InMemoryCache<Cacheable: Identifiable & Sendable>: ObjectCache { + + private var objects: [Cacheable.ID: Cacheable] = [:] + + func get(_ id: Cacheable.ID) -> Cacheable? { + objects[id] + } + + func store(_ object: Cacheable) { + objects[object.id] = object + } + + func store(_ newObjects: [Cacheable]) { + for object in newObjects { + objects[object.id] = object + } + } + + func all() -> [Cacheable] { + [Cacheable](objects.values) + } + + func invalidate(_ id: Cacheable.ID) { + self.objects.removeValue(forKey: id) + } +} + /// 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 { private let client: WordPressClient - private var fetchUsersTask: Task<[DisplayUser], Error>? - - private(set) var users: [DisplayUser]? { - didSet { - if let users { - usersUpdatesContinuation.yield(users) - } - } - } - nonisolated let usersUpdates: AsyncStream<[DisplayUser]> - private nonisolated let usersUpdatesContinuation: AsyncStream<[DisplayUser]>.Continuation + private let cache: any ObjectCache<DisplayUser> private var currentUser: UserWithEditContext? - init(client: WordPressClient) { + private var tasks: [Task<(), any Error>] = [] + + init(client: WordPressClient, cache: any ObjectCache<DisplayUser>) { self.client = client - (usersUpdates, usersUpdatesContinuation) = AsyncStream<[DisplayUser]>.makeStream() + self.cache = cache } deinit { - usersUpdatesContinuation.finish() - fetchUsersTask?.cancel() + for task in tasks { + task.cancel() + } } - func fetchUsers() async throws -> [DisplayUser] { - let users = try await createFetchUsersTaskIfNeeded().value - self.users = users - return users - } + func fetchPaginatedUsers() -> AsyncThrowingStream<[DisplayUser], Error> { + AsyncThrowingStream { continuation in + let handle = Task { + let all = await self.cache.all() - 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)) - .compactMap { DisplayUser(user: $0) } + continuation.yield(all) + + for try await page in await self.client.api + .users + .sequenceWithEditContext(params: UserListParams(perPage: 5)) + .map({ $0.compactMap(DisplayUser.init) }) { + await self.cache.store(page) + continuation.yield(page) + } + + continuation.finish() + } + + self.tasks.append(handle) } - fetchUsersTask = task - return task } func isCurrentUserCapableOf(_ capability: String) async throws -> Bool { @@ -58,7 +90,7 @@ actor UserService: UserServiceProtocol { if let cached = self.currentUser { currentUser = cached } else { - currentUser = try await self.client.api.users.retrieveMeWithEditContext() + currentUser = try await self.client.api.users.retrieveMeWithEditContext().data self.currentUser = currentUser } @@ -69,11 +101,11 @@ actor UserService: UserServiceProtocol { let result = try await client.api.users.delete( userId: id, params: UserDeleteParams(reassign: newUserId) - ) + ).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 { + await cache.invalidate(id) } } @@ -83,7 +115,6 @@ actor UserService: UserServiceProtocol { params: UserUpdateParams(password: newPassword) ) } - } private extension DisplayUser { diff --git a/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+SectionHelpers.swift b/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+SectionHelpers.swift index 65fe07c64dd6..19dc3568d64b 100644 --- a/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+SectionHelpers.swift +++ b/WordPress/Classes/ViewRelated/Blog/Blog Details/BlogDetailsViewController+SectionHelpers.swift @@ -113,7 +113,8 @@ extension BlogDetailsViewController { } @objc func shouldAddUsersRow() -> Bool { - FeatureFlag.selfHostedSiteUserManagement.enabled && blog.isSelfHosted + true +// FeatureFlag.selfHostedSiteUserManagement.enabled && blog.isSelfHosted } @objc func shouldAddPluginsRow() -> Bool { @@ -144,6 +145,8 @@ extension BlogDetailsViewController { } } + static var sharedUserCache = InMemoryCache<DisplayUser>() + @objc func showApplicationPasswordManagement() { guard let presentationDelegate, let userId = self.blog.userID?.intValue else { return @@ -165,8 +168,8 @@ extension BlogDetailsViewController { let feature = NSLocalizedString("applicationPasswordRequired.feature.users", value: "User Management", comment: "Feature name for managing users in the app") let rootView = ApplicationPasswordRequiredView(blog: self.blog, localizedFeatureName: feature) { client in - let service = UserService(client: client) - return UserListView(userService: service) + let userService = UserService(client: client, cache: Self.sharedUserCache) + UserListView(userService: userService) } presentationDelegate.presentBlogDetailsViewController(UIHostingController(rootView: rootView)) }