From d1e2194a1ab1904b6e788906c02872a840044d84 Mon Sep 17 00:00:00 2001 From: Mauro <34335419+Velin92@users.noreply.github.com> Date: Fri, 12 Jan 2024 09:46:51 +0100 Subject: [PATCH] updateMembers now is chain of membersNoSync + membersWithSync (#2324) --- ElementX.xcodeproj/project.pbxproj | 2 +- .../xcshareddata/swiftpm/Package.resolved | 4 +-- .../Mocks/Generated/GeneratedMocks.swift | 22 ++++++------ .../Mocks/NotificationSettingsProxyMock.swift | 10 +++--- ...mNotificationSettingsScreenViewModel.swift | 2 +- ...NotificationSettingsEditScreenModels.swift | 4 +-- ...ificationSettingsEditScreenViewModel.swift | 4 +-- .../View/NotificationSettingsEditScreen.swift | 2 +- .../NotificationSettingsProxy.swift | 4 +-- .../NotificationSettingsProxyProtocol.swift | 2 +- .../Sources/Services/Room/RoomProxy.swift | 36 +++++++++++-------- ...tionSettingsEditScreenViewModelTests.swift | 6 ++-- ...ficationSettingsScreenViewModelTests.swift | 2 +- changelog.d/2304.bugfix | 1 + project.yml | 2 +- 15 files changed, 55 insertions(+), 48 deletions(-) create mode 100644 changelog.d/2304.bugfix diff --git a/ElementX.xcodeproj/project.pbxproj b/ElementX.xcodeproj/project.pbxproj index 3836133a3..a4c3d2bf4 100644 --- a/ElementX.xcodeproj/project.pbxproj +++ b/ElementX.xcodeproj/project.pbxproj @@ -6660,7 +6660,7 @@ repositoryURL = "https://github.com/matrix-org/matrix-rust-components-swift"; requirement = { kind = exactVersion; - version = 1.1.31; + version = 1.1.32; }; }; 821C67C9A7F8CC3FD41B28B4 /* XCRemoteSwiftPackageReference "emojibase-bindings" */ = { diff --git a/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 6632ce575..dd302ba96 100644 --- a/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -130,8 +130,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/matrix-org/matrix-rust-components-swift", "state" : { - "revision" : "c9c6725af2c9fa93c19f710e307b0b25e0f1fa26", - "version" : "1.1.31" + "revision" : "07556e1b475291ec4794e22e9348a5a7db137595", + "version" : "1.1.32" } }, { diff --git a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift index ead2daebc..e5fb1d207 100644 --- a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift +++ b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift @@ -1614,21 +1614,21 @@ class NotificationSettingsProxyMock: NotificationSettingsProxyProtocol { return getRoomsWithUserDefinedRulesReturnValue } } - //MARK: - canHomeserverPushEncryptedEventsToDevice + //MARK: - canPushEncryptedEventsToDevice - var canHomeserverPushEncryptedEventsToDeviceCallsCount = 0 - var canHomeserverPushEncryptedEventsToDeviceCalled: Bool { - return canHomeserverPushEncryptedEventsToDeviceCallsCount > 0 + var canPushEncryptedEventsToDeviceCallsCount = 0 + var canPushEncryptedEventsToDeviceCalled: Bool { + return canPushEncryptedEventsToDeviceCallsCount > 0 } - var canHomeserverPushEncryptedEventsToDeviceReturnValue: Bool! - var canHomeserverPushEncryptedEventsToDeviceClosure: (() async -> Bool)? + var canPushEncryptedEventsToDeviceReturnValue: Bool! + var canPushEncryptedEventsToDeviceClosure: (() async -> Bool)? - func canHomeserverPushEncryptedEventsToDevice() async -> Bool { - canHomeserverPushEncryptedEventsToDeviceCallsCount += 1 - if let canHomeserverPushEncryptedEventsToDeviceClosure = canHomeserverPushEncryptedEventsToDeviceClosure { - return await canHomeserverPushEncryptedEventsToDeviceClosure() + func canPushEncryptedEventsToDevice() async -> Bool { + canPushEncryptedEventsToDeviceCallsCount += 1 + if let canPushEncryptedEventsToDeviceClosure = canPushEncryptedEventsToDeviceClosure { + return await canPushEncryptedEventsToDeviceClosure() } else { - return canHomeserverPushEncryptedEventsToDeviceReturnValue + return canPushEncryptedEventsToDeviceReturnValue } } } diff --git a/ElementX/Sources/Mocks/NotificationSettingsProxyMock.swift b/ElementX/Sources/Mocks/NotificationSettingsProxyMock.swift index a36c480b5..a914fae3c 100644 --- a/ElementX/Sources/Mocks/NotificationSettingsProxyMock.swift +++ b/ElementX/Sources/Mocks/NotificationSettingsProxyMock.swift @@ -22,12 +22,12 @@ struct NotificationSettingsProxyMockConfiguration { var callback = PassthroughSubject() var defaultRoomMode: RoomNotificationModeProxy var roomMode: RoomNotificationSettingsProxyMock - var canHomeserverPushEncryptedEvents = false + var canPushEncryptedEvents = false - init(defaultRoomMode: RoomNotificationModeProxy = .allMessages, roomMode: RoomNotificationModeProxy = .allMessages, canHomeserverPushEncryptedEvents: Bool = false) { + init(defaultRoomMode: RoomNotificationModeProxy = .allMessages, roomMode: RoomNotificationModeProxy = .allMessages, canPushEncryptedEvents: Bool = false) { self.defaultRoomMode = defaultRoomMode self.roomMode = RoomNotificationSettingsProxyMock(with: RoomNotificationSettingsProxyMockConfiguration(mode: roomMode, isDefault: defaultRoomMode == roomMode)) - self.canHomeserverPushEncryptedEvents = canHomeserverPushEncryptedEvents + self.canPushEncryptedEvents = canPushEncryptedEvents } } @@ -92,8 +92,8 @@ extension NotificationSettingsProxyMock { } } - canHomeserverPushEncryptedEventsToDeviceClosure = { - configuration.canHomeserverPushEncryptedEvents + canPushEncryptedEventsToDeviceClosure = { + configuration.canPushEncryptedEvents } } } diff --git a/ElementX/Sources/Screens/RoomNotificationSettingsScreen/RoomNotificationSettingsScreenViewModel.swift b/ElementX/Sources/Screens/RoomNotificationSettingsScreen/RoomNotificationSettingsScreenViewModel.swift index 5290c8a21..e3fe9b2ca 100644 --- a/ElementX/Sources/Screens/RoomNotificationSettingsScreen/RoomNotificationSettingsScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomNotificationSettingsScreen/RoomNotificationSettingsScreenViewModel.swift @@ -84,7 +84,7 @@ class RoomNotificationSettingsScreenViewModel: RoomNotificationSettingsScreenVie } private func fetchRoomNotificationSettings() async { - state.shouldDisplayMentionsOnlyDisclaimer = roomProxy.isEncrypted ? await !notificationSettingsProxy.canHomeserverPushEncryptedEventsToDevice() : false + state.shouldDisplayMentionsOnlyDisclaimer = roomProxy.isEncrypted ? await !notificationSettingsProxy.canPushEncryptedEventsToDevice() : false do { // `isOneToOne` here is not the same as `isDirect` on the room. From the point of view of the push rule, a one-to-one room is a room with exactly two active members. let settings = try await notificationSettingsProxy.getNotificationSettings(roomId: roomProxy.id, diff --git a/ElementX/Sources/Screens/Settings/NotificationSettingsEditScreen/NotificationSettingsEditScreenModels.swift b/ElementX/Sources/Screens/Settings/NotificationSettingsEditScreen/NotificationSettingsEditScreenModels.swift index af095ff71..90456462d 100644 --- a/ElementX/Sources/Screens/Settings/NotificationSettingsEditScreen/NotificationSettingsEditScreenModels.swift +++ b/ElementX/Sources/Screens/Settings/NotificationSettingsEditScreen/NotificationSettingsEditScreenModels.swift @@ -33,7 +33,7 @@ struct NotificationSettingsEditScreenViewState: BindableState { var defaultMode: NotificationSettingsEditScreenDefaultMode? var pendingMode: NotificationSettingsEditScreenDefaultMode? var roomsWithUserDefinedMode: [NotificationSettingsEditScreenRoom] = [] - var canHomeServerPushEncryptedEvents = false + var canPushEncryptedEvents = false func isSelected(mode: NotificationSettingsEditScreenDefaultMode) -> Bool { pendingMode == nil && defaultMode == mode @@ -41,7 +41,7 @@ struct NotificationSettingsEditScreenViewState: BindableState { func description(for mode: NotificationSettingsEditScreenDefaultMode) -> String? { guard mode == .mentionsAndKeywordsOnly, - !canHomeServerPushEncryptedEvents else { + !canPushEncryptedEvents else { return nil } return L10n.screenNotificationSettingsMentionsOnlyDisclaimer diff --git a/ElementX/Sources/Screens/Settings/NotificationSettingsEditScreen/NotificationSettingsEditScreenViewModel.swift b/ElementX/Sources/Screens/Settings/NotificationSettingsEditScreen/NotificationSettingsEditScreenViewModel.swift index 804d099e1..3692b79ba 100644 --- a/ElementX/Sources/Screens/Settings/NotificationSettingsEditScreen/NotificationSettingsEditScreenViewModel.swift +++ b/ElementX/Sources/Screens/Settings/NotificationSettingsEditScreen/NotificationSettingsEditScreenViewModel.swift @@ -93,7 +93,7 @@ class NotificationSettingsEditScreenViewModel: NotificationSettingsEditScreenVie if encrypted_mode == unencrypted_mode { mode = encrypted_mode } - let canHomserverPushEncryptedEvents = await notificationSettingsProxy.canHomeserverPushEncryptedEventsToDevice() + let canPushEncryptedEvents = await notificationSettingsProxy.canPushEncryptedEventsToDevice() guard !Task.isCancelled else { return } switch mode { @@ -104,7 +104,7 @@ class NotificationSettingsEditScreenViewModel: NotificationSettingsEditScreenVie default: state.defaultMode = nil } - state.canHomeServerPushEncryptedEvents = canHomserverPushEncryptedEvents + state.canPushEncryptedEvents = canPushEncryptedEvents } } diff --git a/ElementX/Sources/Screens/Settings/NotificationSettingsEditScreen/View/NotificationSettingsEditScreen.swift b/ElementX/Sources/Screens/Settings/NotificationSettingsEditScreen/View/NotificationSettingsEditScreen.swift index 48e765725..168886a01 100644 --- a/ElementX/Sources/Screens/Settings/NotificationSettingsEditScreen/View/NotificationSettingsEditScreen.swift +++ b/ElementX/Sources/Screens/Settings/NotificationSettingsEditScreen/View/NotificationSettingsEditScreen.swift @@ -114,7 +114,7 @@ struct NotificationSettingsEditScreen_Previews: PreviewProvider, TestablePreview }() static let viewModelGroupChatsWithouDisclaimer: NotificationSettingsEditScreenViewModel = { - let notificationSettingsProxy = NotificationSettingsProxyMock(with: .init(canHomeserverPushEncryptedEvents: true)) + let notificationSettingsProxy = NotificationSettingsProxyMock(with: .init(canPushEncryptedEvents: true)) notificationSettingsProxy.getDefaultRoomNotificationModeIsEncryptedIsOneToOneReturnValue = .allMessages notificationSettingsProxy.getRoomsWithUserDefinedRulesReturnValue = [RoomSummary].mockRooms.compactMap(\.id) diff --git a/ElementX/Sources/Services/NotificationSettings/NotificationSettingsProxy.swift b/ElementX/Sources/Services/NotificationSettings/NotificationSettingsProxy.swift index 065c47597..b1fbcbfa6 100644 --- a/ElementX/Sources/Services/NotificationSettings/NotificationSettingsProxy.swift +++ b/ElementX/Sources/Services/NotificationSettings/NotificationSettingsProxy.swift @@ -141,8 +141,8 @@ final class NotificationSettingsProxy: NotificationSettingsProxyProtocol { await notificationSettings.getRoomsWithUserDefinedRules(enabled: true) } - func canHomeserverPushEncryptedEventsToDevice() async -> Bool { - await notificationSettings.canHomeserverPushEncryptedEventToDevice() + func canPushEncryptedEventsToDevice() async -> Bool { + await notificationSettings.canPushEncryptedEventToDevice() } // MARK: - Private diff --git a/ElementX/Sources/Services/NotificationSettings/NotificationSettingsProxyProtocol.swift b/ElementX/Sources/Services/NotificationSettings/NotificationSettingsProxyProtocol.swift index bb3e4e824..91514222f 100644 --- a/ElementX/Sources/Services/NotificationSettings/NotificationSettingsProxyProtocol.swift +++ b/ElementX/Sources/Services/NotificationSettings/NotificationSettingsProxyProtocol.swift @@ -40,5 +40,5 @@ protocol NotificationSettingsProxyProtocol { func isInviteForMeEnabled() async throws -> Bool func setInviteForMeEnabled(enabled: Bool) async throws func getRoomsWithUserDefinedRules() async throws -> [String] - func canHomeserverPushEncryptedEventsToDevice() async -> Bool + func canPushEncryptedEventsToDevice() async -> Bool } diff --git a/ElementX/Sources/Services/Room/RoomProxy.swift b/ElementX/Sources/Services/Room/RoomProxy.swift index d9c1d47bc..f5f69dbe7 100644 --- a/ElementX/Sources/Services/Room/RoomProxy.swift +++ b/ElementX/Sources/Services/Room/RoomProxy.swift @@ -65,10 +65,6 @@ class RoomProxy: RoomProxyProtocol { pollHistoryTimeline = await TimelineProxy(timeline: room.pollHistory(), backgroundTaskService: backgroundTaskService) Task { - // Force the timeline to load member details so it can populate sender profiles whenever we add a timeline listener - // This should become automatic on the RustSDK side at some point - await room.timeline().fetchMembers() - await updateMembers() } } @@ -179,21 +175,31 @@ class RoomProxy: RoomProxyProtocol { } } } - + func updateMembers() async { + // We always update members first using the no sync API in case internet is not readily available + // To get the members stored on disk first, this API call is very fast. do { - let membersIterator = try await room.members() - guard let members = membersIterator.nextChunk(chunkSize: membersIterator.len()) else { - return + let membersNoSyncIterator = try await room.membersNoSync() + if let members = membersNoSyncIterator.nextChunk(chunkSize: membersNoSyncIterator.len()) { + membersSubject.value = members.map { + RoomMemberProxy(member: $0, backgroundTaskService: self.backgroundTaskService) + } } - - let roomMembersProxies = members.map { - RoomMemberProxy(member: $0, backgroundTaskService: self.backgroundTaskService) - } - - membersSubject.value = roomMembersProxies } catch { - return + MXLog.error("[RoomProxy] Failed to update members using no sync API: \(error)") + } + + do { + // Then we update members using the sync API, this is slower but will get us the latest members + let membersIterator = try await room.members() + if let members = membersIterator.nextChunk(chunkSize: membersIterator.len()) { + membersSubject.value = members.map { + RoomMemberProxy(member: $0, backgroundTaskService: self.backgroundTaskService) + } + } + } catch { + MXLog.error("[RoomProxy] Failed to update members using sync API: \(error)") } } diff --git a/UnitTests/Sources/NotificationSettingsEditScreenViewModelTests.swift b/UnitTests/Sources/NotificationSettingsEditScreenViewModelTests.swift index 58899ca61..d3a63b693 100644 --- a/UnitTests/Sources/NotificationSettingsEditScreenViewModelTests.swift +++ b/UnitTests/Sources/NotificationSettingsEditScreenViewModelTests.swift @@ -72,7 +72,7 @@ class NotificationSettingsEditScreenViewModelTests: XCTestCase { XCTAssertEqual(context.viewState.defaultMode, .mentionsAndKeywordsOnly) XCTAssertNil(context.viewState.bindings.alertInfo) - XCTAssertFalse(context.viewState.canHomeServerPushEncryptedEvents) + XCTAssertFalse(context.viewState.canPushEncryptedEvents) XCTAssertNotNil(context.viewState.description(for: .mentionsAndKeywordsOnly)) } @@ -85,7 +85,7 @@ class NotificationSettingsEditScreenViewModelTests: XCTestCase { return .mentionsAndKeywordsOnly } } - notificationSettingsProxy.canHomeserverPushEncryptedEventsToDeviceClosure = { + notificationSettingsProxy.canPushEncryptedEventsToDeviceClosure = { true } viewModel = NotificationSettingsEditScreenViewModel(chatType: .groupChat, @@ -113,7 +113,7 @@ class NotificationSettingsEditScreenViewModelTests: XCTestCase { XCTAssertEqual(context.viewState.defaultMode, .mentionsAndKeywordsOnly) XCTAssertNil(context.viewState.bindings.alertInfo) - XCTAssertTrue(context.viewState.canHomeServerPushEncryptedEvents) + XCTAssertTrue(context.viewState.canPushEncryptedEvents) XCTAssertNil(context.viewState.description(for: .mentionsAndKeywordsOnly)) } diff --git a/UnitTests/Sources/RoomNotificationSettingsScreenViewModelTests.swift b/UnitTests/Sources/RoomNotificationSettingsScreenViewModelTests.swift index 26c8a25d2..a4f9689d7 100644 --- a/UnitTests/Sources/RoomNotificationSettingsScreenViewModelTests.swift +++ b/UnitTests/Sources/RoomNotificationSettingsScreenViewModelTests.swift @@ -56,7 +56,7 @@ class RoomNotificationSettingsScreenViewModelTests: XCTestCase { func testInitialStateDefaultModeEncryptedRoomWithCanPushEncrypted() async throws { let roomProxyMock = RoomProxyMock(with: .init(displayName: "Test", isEncrypted: true, joinedMembersCount: 0)) - let notificationSettingsProxyMock = NotificationSettingsProxyMock(with: .init(canHomeserverPushEncryptedEvents: true)) + let notificationSettingsProxyMock = NotificationSettingsProxyMock(with: .init(canPushEncryptedEvents: true)) notificationSettingsProxyMock.getNotificationSettingsRoomIdIsEncryptedIsOneToOneReturnValue = RoomNotificationSettingsProxyMock(with: .init(mode: .mentionsAndKeywordsOnly, isDefault: true)) diff --git a/changelog.d/2304.bugfix b/changelog.d/2304.bugfix new file mode 100644 index 000000000..db0758cf9 --- /dev/null +++ b/changelog.d/2304.bugfix @@ -0,0 +1 @@ +Fix for read receipts loading slowly when opening a room. \ No newline at end of file diff --git a/project.yml b/project.yml index 01c63ee3d..55753ea1c 100644 --- a/project.yml +++ b/project.yml @@ -47,7 +47,7 @@ packages: # Element/Matrix dependencies MatrixRustSDK: url: https://github.com/matrix-org/matrix-rust-components-swift - exactVersion: 1.1.31 + exactVersion: 1.1.32 # path: ../matrix-rust-sdk Compound: url: https://github.com/element-hq/compound-ios