letro/letro-ios
1
0
Fork 0
Code Issues Pull Requests Actions 5 Packages Projects Releases 2 Wiki Activity

Stop relying on the NotificationCenter for removing delivered push notifications:

Browse Source 
- this will improve the architecture but more importantly will remove some very flakey unit tests
This commit is contained in:
Stefan Ceriu
2024-04-12 17:46:37 +03:00
committed by Stefan Ceriu
parent 44e1d234a4
commit 4b4c43ba06
15 changed files with 113 additions and 147 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
ElementX.xcodeproj/project.pbxproj
View File

@@ -827,7 +827,6 @@
C413D36D44F89DE63D3ADFA4 /* ReportContentScreen.swift in Sources */ = {isa = PBXBuildFile; fileRef = A433BE28B40D418237BE37B5 /* ReportContentScreen.swift */; };
C4180F418235DAD9DD173951 /* TemplateScreenUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9873076F224E4CE09D8BD47D /* TemplateScreenUITests.swift */; };
C49FCC766673006B6D299F1C /* RoomDetailsEditScreenViewModelProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1DF8F7A3AD83D04C08D75E01 /* RoomDetailsEditScreenViewModelProtocol.swift */; };
C4C84901ABAC9B17564AB7EB /* NotificationName.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4151163F666ED94FD959475A /* NotificationName.swift */; };
C4D2BCAA54E2C62B94B24AF4 /* InviteUsersScreen.swift in Sources */ = {isa = PBXBuildFile; fileRef = C2E9B841EE4878283ECDB554 /* InviteUsersScreen.swift */; };
C4E0D03DF88242697545A9B7 /* UserIndicatorController.swift in Sources */ = {isa = PBXBuildFile; fileRef = FD1275D9CE0FFBA6E8E85426 /* UserIndicatorController.swift */; };
C4F69156C31A447FEFF2A47C /* DTHTMLElement+AttributedStringBuilder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1E508AB0EDEE017FF4F6F8D1 /* DTHTMLElement+AttributedStringBuilder.swift */; };
@@ -1396,7 +1395,6 @@
40076C770A5FB83325252973 /* VoiceMessageMediaManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VoiceMessageMediaManager.swift; sourceTree = "<group>"; };
40316EFFEAC7B206EE9A55AE /* SecureBackupScreenViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SecureBackupScreenViewModelTests.swift; sourceTree = "<group>"; };
40B21E611DADDEF00307E7AC /* String.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = String.swift; sourceTree = "<group>"; };
4151163F666ED94FD959475A /* NotificationName.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationName.swift; sourceTree = "<group>"; };
4176C3E20C772DE8D182863C /* LegalInformationScreen.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LegalInformationScreen.swift; sourceTree = "<group>"; };
41BB37D96C3EA18F3CE8675D /* RoomDirectorySearchScreenModels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomDirectorySearchScreenModels.swift; sourceTree = "<group>"; };
41D041A857614A9AE13C7795 /* RoomChangePermissionsScreenViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomChangePermissionsScreenViewModelTests.swift; sourceTree = "<group>"; };
@@ -3009,7 +3007,6 @@
045253F9967A535EE5B16691 /* Label.swift */,
C14D83B2B7CD5501A0089EFC /* LayoutDirection.swift */,
23AA3F4B285570805CB0CCDD /* MapTiler.swift */,
4151163F666ED94FD959475A /* NotificationName.swift */,
F72EFC8C634469F9262659C7 /* NSItemProvider.swift */,
95BAC0F6C9644336E9567EE6 /* NSRegularExpresion.swift */,
62B07B296D7A9D2F09120853 /* OrderedSet.swift */,
@@ -6201,7 +6198,6 @@
FD4C21F8DA1E273DE94FCD1A /* NotificationItemProxyProtocol.swift in Sources */,
652ACCF104A8CEF30788963C /* NotificationManager.swift in Sources */,
06D3942496E9E0E655F14D21 /* NotificationManagerProtocol.swift in Sources */,
C4C84901ABAC9B17564AB7EB /* NotificationName.swift in Sources */,
5139F4BD5A5DF6F8D11A9BDE /* NotificationPermissionsScreen.swift in Sources */,
454311EAC17D778E19F46592 /* NotificationPermissionsScreenCoordinator.swift in Sources */,
3E7B65C2C97748D5D65AAA8B /* NotificationPermissionsScreenModels.swift in Sources */,

10
ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift
View File

@@ -32,6 +32,7 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
private let bugReportService: BugReportServiceProtocol
private let appSettings: AppSettings
private let analytics: AnalyticsService
private let notificationManager: NotificationManagerProtocol
private let stateMachine: UserSessionFlowCoordinatorStateMachine
@@ -82,6 +83,7 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
self.roomTimelineControllerFactory = roomTimelineControllerFactory
self.appSettings = appSettings
self.analytics = analytics
self.notificationManager = notificationManager
navigationSplitCoordinator = NavigationSplitCoordinator(placeholderCoordinator: PlaceholderScreenCoordinator())
@@ -467,7 +469,9 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
}
Task {
await userSession.clientProxy.trackRecentlyVisitedRoom(roomID)
let _ = await userSession.clientProxy.trackRecentlyVisitedRoom(roomID)
await notificationManager.removeDeliveredMessageNotifications(for: roomID)
}
}
@@ -527,6 +531,10 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
sidebarNavigationStackCoordinator.push(coordinator, animated: animated) { [weak self] in
self?.stateMachine.processEvent(.dismissedInvitesScreen)
}
Task {
await notificationManager.removeDeliveredInviteNotifications()
}
}
private func dismissInvitesList(animated: Bool) {

74
ElementX/Sources/Mocks/Generated/GeneratedMocks.swift
View File

@@ -5329,6 +5329,80 @@ class NotificationManagerMock: NotificationManagerProtocol {
requestAuthorizationCallsCount += 1
requestAuthorizationClosure?()
}
//MARK: - removeDeliveredMessageNotifications
var removeDeliveredMessageNotificationsForUnderlyingCallsCount = 0
var removeDeliveredMessageNotificationsForCallsCount: Int {
get {
if Thread.isMainThread {
return removeDeliveredMessageNotificationsForUnderlyingCallsCount
} else {
var returnValue: Int? = nil
DispatchQueue.main.sync {
returnValue = removeDeliveredMessageNotificationsForUnderlyingCallsCount
}
return returnValue!
}
}
set {
if Thread.isMainThread {
removeDeliveredMessageNotificationsForUnderlyingCallsCount = newValue
} else {
DispatchQueue.main.sync {
removeDeliveredMessageNotificationsForUnderlyingCallsCount = newValue
}
}
}
}
var removeDeliveredMessageNotificationsForCalled: Bool {
return removeDeliveredMessageNotificationsForCallsCount > 0
}
var removeDeliveredMessageNotificationsForReceivedRoomID: String?
var removeDeliveredMessageNotificationsForReceivedInvocations: [String] = []
var removeDeliveredMessageNotificationsForClosure: ((String) async -> Void)?
func removeDeliveredMessageNotifications(for roomID: String) async {
removeDeliveredMessageNotificationsForCallsCount += 1
removeDeliveredMessageNotificationsForReceivedRoomID = roomID
removeDeliveredMessageNotificationsForReceivedInvocations.append(roomID)
await removeDeliveredMessageNotificationsForClosure?(roomID)
}
//MARK: - removeDeliveredInviteNotifications
var removeDeliveredInviteNotificationsUnderlyingCallsCount = 0
var removeDeliveredInviteNotificationsCallsCount: Int {
get {
if Thread.isMainThread {
return removeDeliveredInviteNotificationsUnderlyingCallsCount
} else {
var returnValue: Int? = nil
DispatchQueue.main.sync {
returnValue = removeDeliveredInviteNotificationsUnderlyingCallsCount
}
return returnValue!
}
}
set {
if Thread.isMainThread {
removeDeliveredInviteNotificationsUnderlyingCallsCount = newValue
} else {
DispatchQueue.main.sync {
removeDeliveredInviteNotificationsUnderlyingCallsCount = newValue
}
}
}
}
var removeDeliveredInviteNotificationsCalled: Bool {
return removeDeliveredInviteNotificationsCallsCount > 0
}
var removeDeliveredInviteNotificationsClosure: (() async -> Void)?
func removeDeliveredInviteNotifications() async {
removeDeliveredInviteNotificationsCallsCount += 1
await removeDeliveredInviteNotificationsClosure?()
}
}
class NotificationSettingsProxyMock: NotificationSettingsProxyProtocol {
var callbacks: PassthroughSubject<NotificationSettingsProxyCallback, Never> {

22
ElementX/Sources/Other/Extensions/NotificationName.swift
View File

@@ -1,22 +0,0 @@
//
// Copyright 2023 New Vector Ltd
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
import Foundation
extension Notification.Name {
static let roomMarkedAsRead = Notification.Name("roomMarkedAsRead")
static let invitesScreenAppeared = Notification.Name("invitesScreenAppeared")
}

1
ElementX/Sources/Screens/InvitesScreen/InvitesScreenModels.swift
View File

@@ -43,5 +43,4 @@ struct InvitesScreenRoomDetails: Identifiable {
enum InvitesScreenViewAction {
case accept(InvitesScreenRoomDetails)
case decline(InvitesScreenRoomDetails)
case appeared
}

2
ElementX/Sources/Screens/InvitesScreen/InvitesScreenViewModel.swift
View File

@@ -57,8 +57,6 @@ class InvitesScreenViewModel: InvitesScreenViewModelType, InvitesScreenViewModel
accept(invite: invite)
case .decline(let invite):
startDeclineFlow(invite: invite)
case .appeared:
notificationCenterProtocol.post(name: .invitesScreenAppeared, object: nil)
}
}

3
ElementX/Sources/Screens/InvitesScreen/View/InvitesScreen.swift
View File

@@ -45,9 +45,6 @@ struct InvitesScreen: View {
noInvitesContent
}
}
.onAppear {
context.send(viewAction: .appeared)
}
}
private var noInvitesContent: some View {

2
ElementX/Sources/Screens/JoinRoomScreen/JoinRoomScreenViewModel.swift
View File

@@ -60,7 +60,7 @@ class JoinRoomScreenViewModel: JoinRoomScreenViewModelType, JoinRoomScreenViewMo
switch await clientProxy.joinRoom(state.roomID) {
case .success:
actionsSubject.send(.joined)
case .failure(let error):
case .failure:
state.bindings.alertInfo = .init(id: .joinFailed)
}
}

7
ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift
View File

@@ -415,12 +415,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
private func sendReadReceiptIfNeeded(for lastVisibleItemID: TimelineItemIdentifier) async {
guard application.applicationState == .active else { return }
// Clear any notifications from notification center.
if lastVisibleItemID.timelineID == state.timelineViewState.timelineIDs.last {
notificationCenter.post(name: .roomMarkedAsRead, object: roomProxy.id)
}
await timelineController.sendReadReceipt(for: lastVisibleItemID)
}

62
ElementX/Sources/Services/Notification/Manager/NotificationManager.swift
View File

@@ -33,7 +33,6 @@ final class NotificationManager: NSObject, NotificationManagerProtocol {
self.notificationCenter = notificationCenter
self.appSettings = appSettings
super.init()
addObservers()
}
// MARK: NotificationManagerProtocol
@@ -100,8 +99,8 @@ final class NotificationManager: NSObject, NotificationManagerProtocol {
guard let self else { return }
if await notificationCenter.authorizationStatus() == .authorized {
await MainActor.run {
delegate?.registerForRemoteNotifications()
await MainActor.run { [weak self] in
self?.delegate?.registerForRemoteNotifications()
}
}
}
@@ -128,6 +127,22 @@ final class NotificationManager: NSObject, NotificationManagerProtocol {
}
}
func removeDeliveredMessageNotifications(for roomID: String) async {
let notificationsIdentifiers = await notificationCenter
.deliveredNotifications()
.filter { $0.request.content.roomID == roomID }
.map(\.request.identifier)
notificationCenter.removeDeliveredNotifications(withIdentifiers: notificationsIdentifiers)
}
func removeDeliveredInviteNotifications() async {
let notificationsIdentifiers = await notificationCenter
.deliveredNotifications()
.filter { $0.request.content.categoryIdentifier == NotificationConstants.Category.invite }
.map(\.request.identifier)
notificationCenter.removeDeliveredNotifications(withIdentifiers: notificationsIdentifiers)
}
private func setPusher(with deviceToken: Data, clientProxy: ClientProxyProtocol) async -> Bool {
do {
let defaultPayload = APNSPayload(aps: APSInfo(mutableContent: 1,
@@ -177,47 +192,6 @@ final class NotificationManager: NSObject, NotificationManagerProtocol {
delegate?.unregisterForRemoteNotifications()
}
}
private func addObservers() {
NotificationCenter
.default
.publisher(for: .roomMarkedAsRead)
.sink { [weak self] notification in
guard let roomID = notification.object as? String else {
return
}
self?.removeMessageNotifications(for: roomID)
}
.store(in: &cancellables)
NotificationCenter
.default
.publisher(for: .invitesScreenAppeared)
.sink { [weak self] _ in
self?.removeInviteNotifications()
}
.store(in: &cancellables)
}
private func removeMessageNotifications(for roomID: String) {
Task {
let notificationsIdentifiers = await notificationCenter
.deliveredNotifications()
.filter { $0.request.content.roomID == roomID }
.map(\.request.identifier)
notificationCenter.removeDeliveredNotifications(withIdentifiers: notificationsIdentifiers)
}
}
private func removeInviteNotifications() {
Task {
let notificationsIdentifiers = await notificationCenter
.deliveredNotifications()
.filter { $0.request.content.categoryIdentifier == NotificationConstants.Category.invite }
.map(\.request.identifier)
notificationCenter.removeDeliveredNotifications(withIdentifiers: notificationsIdentifiers)
}
}
}
// MARK: - UNUserNotificationCenterDelegate

3
ElementX/Sources/Services/Notification/Manager/NotificationManagerProtocol.swift
View File

@@ -40,4 +40,7 @@ protocol NotificationManagerProtocol: AnyObject {
func setUserSession(_ userSession: UserSessionProtocol?)
func requestAuthorization()
func removeDeliveredMessageNotifications(for roomID: String) async
func removeDeliveredInviteNotifications() async
}

8
UnitTests/Sources/InvitesScreenViewModelTests.swift
View File

@@ -85,14 +85,6 @@ class InvitesScreenViewModelTests: XCTestCase {
context.send(viewAction: .decline(.init(roomDetails: details, isUnread: false)))
XCTAssertNotNil(context.alertInfo)
}
func testHasAppeared() async {
setupViewModel()
context.send(viewAction: .appeared)
await Task.yield()
XCTAssertEqual(mockNotificationCenter.postNameObjectReceivedArguments?.aName, .invitesScreenAppeared)
}
// MARK: - Private

49
UnitTests/Sources/NotificationManager/NotificationManagerTests.swift
View File

@@ -204,55 +204,6 @@ final class NotificationManagerTests: XCTestCase {
await notificationManager.userNotificationCenter(UNUserNotificationCenter.current(), didReceive: response)
XCTAssertTrue(notificationTappedDelegateCalled)
}
func test_MessageNotificationsRemoval() async throws {
notificationCenter.deliveredNotificationsClosure = {
XCTFail("deliveredNotifications should not be called if the object is nil or of the wrong type")
return []
}
// No interaction if the object is nil or of the wrong type
NotificationCenter.default.post(name: .roomMarkedAsRead, object: nil)
try? await Task.sleep(for: .seconds(0.25)) // Wait for the notification to be delivered
XCTAssertEqual(notificationCenter.deliveredNotificationsCallsCount, 0)
XCTAssertEqual(notificationCenter.removeDeliveredNotificationsWithIdentifiersCallsCount, 0)
NotificationCenter.default.post(name: .roomMarkedAsRead, object: 1)
try? await Task.sleep(for: .seconds(0.25)) // Wait for the notification to be delivered
XCTAssertEqual(notificationCenter.deliveredNotificationsCallsCount, 0)
XCTAssertEqual(notificationCenter.removeDeliveredNotificationsWithIdentifiersCallsCount, 0)
// The center calls the delivered and the removal functions when an id is passed
notificationCenter.deliveredNotificationsClosure = nil
notificationCenter.deliveredNotificationsReturnValue = []
let expectation = expectation(description: "Notification should be removed")
notificationCenter.removeDeliveredNotificationsWithIdentifiersClosure = { _ in
expectation.fulfill()
}
NotificationCenter.default.post(name: .roomMarkedAsRead, object: "RoomID")
try? await Task.sleep(for: .seconds(0.25)) // Wait for the notification to be delivered
await fulfillment(of: [expectation])
XCTAssertEqual(notificationCenter.deliveredNotificationsCallsCount, 1)
XCTAssertEqual(notificationCenter.removeDeliveredNotificationsWithIdentifiersCallsCount, 1)
}
func test_InvitesNotificationsRemoval() async {
let expectation = expectation(description: "Notification should be removed")
notificationCenter.deliveredNotificationsReturnValue = []
notificationCenter.removeDeliveredNotificationsWithIdentifiersClosure = { _ in
expectation.fulfill()
}
NotificationCenter.default.post(name: .invitesScreenAppeared, object: nil)
try? await Task.sleep(for: .seconds(0.25)) // Wait for the notification to be delivered
await fulfillment(of: [expectation])
XCTAssertEqual(notificationCenter.deliveredNotificationsCallsCount, 1)
XCTAssertEqual(notificationCenter.removeDeliveredNotificationsWithIdentifiersCallsCount, 1)
}
}
extension NotificationManagerTests: NotificationManagerDelegate {

5
UnitTests/Sources/RoomScreenViewModelTests.swift
View File

@@ -336,11 +336,6 @@ class RoomScreenViewModelTests: XCTestCase {
let arguments = timelineProxy.sendReadReceiptForTypeReceivedArguments
XCTAssertEqual(arguments?.eventID, "t3")
XCTAssertEqual(arguments?.type, .read)
// And the notifications should be cleared.
XCTAssertEqual(notificationCenter.postNameObjectReceivedArguments?.aName, .roomMarkedAsRead)
let roomID = notificationCenter.postNameObjectReceivedArguments?.anObject as? String
XCTAssertEqual(roomID, roomProxy.id)
}
func testSendMoreReadReceipts() async throws {

8
UnitTests/Sources/UserSessionFlowCoordinatorTests.swift
View File

@@ -23,6 +23,8 @@ import Combine
class UserSessionFlowCoordinatorTests: XCTestCase {
var userSessionFlowCoordinator: UserSessionFlowCoordinator!
var navigationRootCoordinator: NavigationRootCoordinator!
var notificationManager: NotificationManagerMock!
var cancellables = Set<AnyCancellable>()
var detailCoordinator: CoordinatorProtocol? {
@@ -45,6 +47,8 @@ class UserSessionFlowCoordinatorTests: XCTestCase {
navigationRootCoordinator = NavigationRootCoordinator()
notificationManager = NotificationManagerMock()
userSessionFlowCoordinator = UserSessionFlowCoordinator(userSession: userSession,
navigationRootCoordinator: navigationRootCoordinator,
windowManager: WindowManagerMock(),
@@ -53,7 +57,7 @@ class UserSessionFlowCoordinatorTests: XCTestCase {
roomTimelineControllerFactory: MockRoomTimelineControllerFactory(),
appSettings: ServiceLocator.shared.settings,
analytics: ServiceLocator.shared.analytics,
notificationManager: NotificationManagerMock(),
notificationManager: notificationManager,
isNewLogin: false)
let deferred = deferFulfillment(userSessionFlowCoordinator.statePublisher) { $0 == .roomList(selectedRoomID: nil) }
@@ -81,6 +85,8 @@ class UserSessionFlowCoordinatorTests: XCTestCase {
try await process(route: .roomList, expectedState: .roomList(selectedRoomID: nil))
XCTAssertNil(detailNavigationStack?.rootCoordinator)
XCTAssertNil(detailCoordinator)
XCTAssertEqual(notificationManager.removeDeliveredMessageNotificationsForReceivedInvocations, ["1", "1", "2"])
}
func testRoomDetailsPresentation() async throws {