From 698f13fc953c9e299cc34c136a154977b9d2d371 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 10 Oct 2024 22:58:23 +0200 Subject: [PATCH] Refactor Core Data usage in RemoteMessagingStore (#3425) Task/Issue URL: https://app.asana.com/0/1201048563534612/1208493869353425/f Description: This change updates RMF client to use new async API from BSK's RemoteMessagingStore. --- DuckDuckGo.xcodeproj/project.pbxproj | 2 +- .../xcshareddata/swiftpm/Package.resolved | 4 +- DuckDuckGo/HomeMessageView.swift | 12 ++-- DuckDuckGo/HomeMessageViewModel.swift | 26 ++++----- DuckDuckGo/HomeMessageViewModelBuilder.swift | 2 +- .../HomeMessageViewSectionRenderer.swift | 8 ++- DuckDuckGo/HomePageConfiguration.swift | 9 ++- .../HomePageMessagesConfiguration.swift | 2 +- DuckDuckGo/NewTabPageMessagesModel.swift | 17 +++--- .../NewTabPageMessagesModelTests.swift | 56 +++++++++---------- 10 files changed, 74 insertions(+), 64 deletions(-) diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 08f726f548..c25fcbd87a 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -10947,7 +10947,7 @@ repositoryURL = "https://github.com/DuckDuckGo/BrowserServicesKit"; requirement = { kind = exactVersion; - version = 198.4.0; + version = 199.0.0; }; }; 9F8FE9472BAE50E50071E372 /* XCRemoteSwiftPackageReference "lottie-spm" */ = { diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 800f6c30e7..7e03d15de5 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -32,8 +32,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/DuckDuckGo/BrowserServicesKit", "state" : { - "revision" : "c642e92389688016e2dad549f6d6e7c4cd1642d3", - "version" : "198.4.0" + "revision" : "ec46d991838527dd8c7041a3673428c0e18e0fdc", + "version" : "199.0.0" } }, { diff --git a/DuckDuckGo/HomeMessageView.swift b/DuckDuckGo/HomeMessageView.swift index a4067808fa..87f61b5849 100644 --- a/DuckDuckGo/HomeMessageView.swift +++ b/DuckDuckGo/HomeMessageView.swift @@ -103,7 +103,9 @@ struct HomeMessageView: View { private var closeButton: some View { Button { - viewModel.onDidClose(.close) + Task { + await viewModel.onDidClose(.close) + } } label: { Image("Close-24") .foregroundColor(.primary) @@ -143,9 +145,11 @@ struct HomeMessageView: View { private var buttons: some View { ForEach(viewModel.buttons, id: \.title) { buttonModel in Button { - buttonModel.action() - if case .share(let value, let title) = buttonModel.actionStyle { - activityItem = ShareItem(value: value, title: title) + Task { @MainActor in + await buttonModel.action() + if case .share(let value, let title) = buttonModel.actionStyle { + activityItem = ShareItem(value: value, title: title) + } } } label: { HStack { diff --git a/DuckDuckGo/HomeMessageViewModel.swift b/DuckDuckGo/HomeMessageViewModel.swift index e728fa1fc6..67acfb3af4 100644 --- a/DuckDuckGo/HomeMessageViewModel.swift +++ b/DuckDuckGo/HomeMessageViewModel.swift @@ -118,40 +118,40 @@ struct HomeMessageViewModel { } } - let onDidClose: (ButtonAction?) -> Void + let onDidClose: (ButtonAction?) async -> Void let onDidAppear: () -> Void let onAttachAdditionalParameters: ((_ useCase: PrivacyProDataReportingUseCase, _ params: [String: String]) -> [String: String])? func mapActionToViewModel(remoteAction: RemoteAction, buttonAction: HomeMessageViewModel.ButtonAction, - onDidClose: @escaping (HomeMessageViewModel.ButtonAction?) -> Void) -> () -> Void { + onDidClose: @escaping (HomeMessageViewModel.ButtonAction?) async -> Void) -> () async -> Void { switch remoteAction { case .share: - return { - onDidClose(buttonAction) + return { @MainActor in + await onDidClose(buttonAction) } case .url(let value): - return { + return { @MainActor in LaunchTabNotification.postLaunchTabNotification(urlString: value) - onDidClose(buttonAction) + await onDidClose(buttonAction) } case .survey(let value): - return { + return { @MainActor in LaunchTabNotification.postLaunchTabNotification(urlString: value) - onDidClose(buttonAction) + await onDidClose(buttonAction) } case .appStore: - return { + return { @MainActor in let url = URL.appStore if UIApplication.shared.canOpenURL(url as URL) { UIApplication.shared.open(url) } - onDidClose(buttonAction) + await onDidClose(buttonAction) } case .dismiss: - return { - onDidClose(buttonAction) + return { @MainActor in + await onDidClose(buttonAction) } } } @@ -166,6 +166,6 @@ struct HomeMessageButtonViewModel { let title: String var actionStyle: ActionStyle = .default - let action: () -> Void + let action: () async -> Void } diff --git a/DuckDuckGo/HomeMessageViewModelBuilder.swift b/DuckDuckGo/HomeMessageViewModelBuilder.swift index e513ef73d8..8600f161f6 100644 --- a/DuckDuckGo/HomeMessageViewModelBuilder.swift +++ b/DuckDuckGo/HomeMessageViewModelBuilder.swift @@ -34,7 +34,7 @@ struct HomeMessageViewModelBuilder { static func build(for remoteMessage: RemoteMessageModel, with privacyProDataReporter: PrivacyProDataReporting?, - onDidClose: @escaping (HomeMessageViewModel.ButtonAction?) -> Void, + onDidClose: @escaping (HomeMessageViewModel.ButtonAction?) async -> Void, onDidAppear: @escaping () -> Void) -> HomeMessageViewModel? { guard let content = remoteMessage.content else { return nil } diff --git a/DuckDuckGo/HomeMessageViewSectionRenderer.swift b/DuckDuckGo/HomeMessageViewSectionRenderer.swift index 3048d865ae..4926c5e3b7 100644 --- a/DuckDuckGo/HomeMessageViewSectionRenderer.swift +++ b/DuckDuckGo/HomeMessageViewSectionRenderer.swift @@ -184,9 +184,11 @@ class HomeMessageViewSectionRenderer: NSObject, HomeViewSectionRenderer { private func dismissHomeMessage(_ message: HomeMessage, at indexPath: IndexPath, in collectionView: UICollectionView) { - homePageConfiguration.dismissHomeMessage(message) - animateCellDismissal(at: indexPath, in: collectionView) { - self.controller?.homeMessageRenderer(self, didDismissHomeMessage: message) + Task { @MainActor in + await homePageConfiguration.dismissHomeMessage(message) + animateCellDismissal(at: indexPath, in: collectionView) { + self.controller?.homeMessageRenderer(self, didDismissHomeMessage: message) + } } } diff --git a/DuckDuckGo/HomePageConfiguration.swift b/DuckDuckGo/HomePageConfiguration.swift index bdec04070c..f476642bfc 100644 --- a/DuckDuckGo/HomePageConfiguration.swift +++ b/DuckDuckGo/HomePageConfiguration.swift @@ -84,11 +84,12 @@ final class HomePageConfiguration: HomePageMessagesConfiguration { return .remoteMessage(remoteMessage: remoteMessageToPresent) } - func dismissHomeMessage(_ homeMessage: HomeMessage) { + @MainActor + func dismissHomeMessage(_ homeMessage: HomeMessage) async { switch homeMessage { case .remoteMessage(let remoteMessage): Logger.remoteMessaging.info("Home message dismissed: \(remoteMessage.id)") - remoteMessagingClient.store.dismissRemoteMessage(withID: remoteMessage.id) + await remoteMessagingClient.store.dismissRemoteMessage(withID: remoteMessage.id) if let index = homeMessages.firstIndex(of: homeMessage) { homeMessages.remove(at: index) @@ -113,7 +114,9 @@ final class HomePageConfiguration: HomePageMessagesConfiguration { Pixel.fire(pixel: .remoteMessageShownUnique, withAdditionalParameters: additionalParameters(for: remoteMessage.id)) } - remoteMessagingClient.store.updateRemoteMessage(withID: remoteMessage.id, asShown: true) + Task { + await remoteMessagingClient.store.updateRemoteMessage(withID: remoteMessage.id, asShown: true) + } } default: diff --git a/DuckDuckGo/HomePageMessagesConfiguration.swift b/DuckDuckGo/HomePageMessagesConfiguration.swift index 44af8eac7f..6dbee5210b 100644 --- a/DuckDuckGo/HomePageMessagesConfiguration.swift +++ b/DuckDuckGo/HomePageMessagesConfiguration.swift @@ -24,6 +24,6 @@ protocol HomePageMessagesConfiguration { func refresh() - func dismissHomeMessage(_ homeMessage: HomeMessage) + func dismissHomeMessage(_ homeMessage: HomeMessage) async func didAppear(_ homeMessage: HomeMessage) } diff --git a/DuckDuckGo/NewTabPageMessagesModel.swift b/DuckDuckGo/NewTabPageMessagesModel.swift index 6be35faf8e..505bc097b5 100644 --- a/DuckDuckGo/NewTabPageMessagesModel.swift +++ b/DuckDuckGo/NewTabPageMessagesModel.swift @@ -53,8 +53,9 @@ final class NewTabPageMessagesModel: ObservableObject { refresh() } - func dismissHomeMessage(_ homeMessage: HomeMessage) { - homePageMessagesConfiguration.dismissHomeMessage(homeMessage) + @MainActor + func dismissHomeMessage(_ homeMessage: HomeMessage) async { + await homePageMessagesConfiguration.dismissHomeMessage(homeMessage) updateHomeMessageViewModel() } @@ -77,7 +78,7 @@ final class NewTabPageMessagesModel: ObservableObject { switch message { case .placeholder: return HomeMessageViewModel(messageId: "", sendPixels: false, modelType: .small(titleText: "", descriptionText: "")) { [weak self] _ in - self?.dismissHomeMessage(message) + await self?.dismissHomeMessage(message) } onDidAppear: { // no-op } onAttachAdditionalParameters: { _, params in @@ -89,7 +90,7 @@ final class NewTabPageMessagesModel: ObservableObject { // as a result of refreshing a config while the user was on a new tab page already. didAppear(message) - return HomeMessageViewModelBuilder.build(for: remoteMessage, with: privacyProDataReporter) { [weak self] action in + return HomeMessageViewModelBuilder.build(for: remoteMessage, with: privacyProDataReporter) { @MainActor [weak self] action in guard let action, let self else { return } @@ -97,7 +98,7 @@ final class NewTabPageMessagesModel: ObservableObject { case .action(let isSharing): if !isSharing { - self.dismissHomeMessage(message) + await self.dismissHomeMessage(message) } if remoteMessage.isMetricsEnabled { pixelFiring.fire(.remoteMessageActionClicked, @@ -106,7 +107,7 @@ final class NewTabPageMessagesModel: ObservableObject { case .primaryAction(let isSharing): if !isSharing { - self.dismissHomeMessage(message) + await self.dismissHomeMessage(message) } if remoteMessage.isMetricsEnabled { pixelFiring.fire(.remoteMessagePrimaryActionClicked, @@ -115,7 +116,7 @@ final class NewTabPageMessagesModel: ObservableObject { case .secondaryAction(let isSharing): if !isSharing { - self.dismissHomeMessage(message) + await self.dismissHomeMessage(message) } if remoteMessage.isMetricsEnabled { pixelFiring.fire(.remoteMessageSecondaryActionClicked, @@ -123,7 +124,7 @@ final class NewTabPageMessagesModel: ObservableObject { } case .close: - self.dismissHomeMessage(message) + await self.dismissHomeMessage(message) if remoteMessage.isMetricsEnabled { pixelFiring.fire(.remoteMessageDismissed, withAdditionalParameters: self.additionalParameters(for: remoteMessage.id)) diff --git a/DuckDuckGoTests/NewTabPageMessagesModelTests.swift b/DuckDuckGoTests/NewTabPageMessagesModelTests.swift index a37e7a0984..617973b4f4 100644 --- a/DuckDuckGoTests/NewTabPageMessagesModelTests.swift +++ b/DuckDuckGoTests/NewTabPageMessagesModelTests.swift @@ -55,7 +55,7 @@ final class NewTabPageMessagesModelTests: XCTestCase { // MARK: Callbacks - func testCallsDismissOnClose() throws { + func testCallsDismissOnClose() async throws { let sut = createSUT() messagesConfiguration.homeMessages = [ .mockRemote(withType: .small(titleText: "", descriptionText: "")), @@ -64,12 +64,12 @@ final class NewTabPageMessagesModelTests: XCTestCase { let model = try XCTUnwrap(sut.homeMessageViewModels.first) - model.onDidClose(.close) + await model.onDidClose(.close) XCTAssertEqual(messagesConfiguration.lastDismissedHomeMessage, messagesConfiguration.homeMessages.first) } - func testCallsDismissOnAction() throws { + func testCallsDismissOnAction() async throws { let sut = createSUT() messagesConfiguration.homeMessages = [ .mockRemote(withType: .small(titleText: "", descriptionText: "")), @@ -78,12 +78,12 @@ final class NewTabPageMessagesModelTests: XCTestCase { let model = try XCTUnwrap(sut.homeMessageViewModels.first) - model.onDidClose(.action(isShare: false)) + await model.onDidClose(.action(isShare: false)) XCTAssertEqual(messagesConfiguration.lastDismissedHomeMessage, messagesConfiguration.homeMessages.first) } - func testCallsDismissOnPrimaryAction() throws { + func testCallsDismissOnPrimaryAction() async throws { let sut = createSUT() messagesConfiguration.homeMessages = [ .mockRemote(withType: .small(titleText: "", descriptionText: "")), @@ -92,12 +92,12 @@ final class NewTabPageMessagesModelTests: XCTestCase { let model = try XCTUnwrap(sut.homeMessageViewModels.first) - model.onDidClose(.primaryAction(isShare: false)) + await model.onDidClose(.primaryAction(isShare: false)) XCTAssertEqual(messagesConfiguration.lastDismissedHomeMessage, messagesConfiguration.homeMessages.first) } - func testCallsDismissOnSecondaryAction() throws { + func testCallsDismissOnSecondaryAction() async throws { let sut = createSUT() messagesConfiguration.homeMessages = [ .mockRemote(withType: .small(titleText: "", descriptionText: "")), @@ -106,12 +106,12 @@ final class NewTabPageMessagesModelTests: XCTestCase { let model = try XCTUnwrap(sut.homeMessageViewModels.first) - model.onDidClose(.secondaryAction(isShare: false)) + await model.onDidClose(.secondaryAction(isShare: false)) XCTAssertEqual(messagesConfiguration.lastDismissedHomeMessage, messagesConfiguration.homeMessages.first) } - func testDoesNotCallDismissWhenSharing() throws { + func testDoesNotCallDismissWhenSharing() async throws { let sut = createSUT() messagesConfiguration.homeMessages = [ .mockRemote(withType: .small(titleText: "", descriptionText: "")), @@ -120,16 +120,16 @@ final class NewTabPageMessagesModelTests: XCTestCase { let model = try XCTUnwrap(sut.homeMessageViewModels.first) - model.onDidClose(.action(isShare: true)) - model.onDidClose(.primaryAction(isShare: true)) - model.onDidClose(.secondaryAction(isShare: true)) + await model.onDidClose(.action(isShare: true)) + await model.onDidClose(.primaryAction(isShare: true)) + await model.onDidClose(.secondaryAction(isShare: true)) XCTAssertNil(messagesConfiguration.lastDismissedHomeMessage) } // MARK: Pixels - func testFiresPixelOnClose() throws { + func testFiresPixelOnClose() async throws { let sut = createSUT() messagesConfiguration.homeMessages = [ .mockRemote(withType: .small(titleText: "", descriptionText: "")), @@ -138,13 +138,13 @@ final class NewTabPageMessagesModelTests: XCTestCase { let model = try XCTUnwrap(sut.homeMessageViewModels.first) - model.onDidClose(.close) + await model.onDidClose(.close) XCTAssertEqual(PixelFiringMock.lastPixel, .remoteMessageDismissed) XCTAssertEqual(PixelFiringMock.lastParams, [PixelParameters.message: "foo"]) } - func testFiresPixelOnAction() throws { + func testFiresPixelOnAction() async throws { let sut = createSUT() messagesConfiguration.homeMessages = [ .mockRemote(withType: .small(titleText: "", descriptionText: "")), @@ -152,13 +152,13 @@ final class NewTabPageMessagesModelTests: XCTestCase { sut.load() let model = try XCTUnwrap(sut.homeMessageViewModels.first) - model.onDidClose(.action(isShare: false)) + await model.onDidClose(.action(isShare: false)) XCTAssertEqual(PixelFiringMock.lastPixel, .remoteMessageActionClicked) XCTAssertEqual(PixelFiringMock.lastParams, [PixelParameters.message: "foo"]) } - func testFiresPixelOnPrimaryAction() throws { + func testFiresPixelOnPrimaryAction() async throws { let sut = createSUT() messagesConfiguration.homeMessages = [ .mockRemote(withType: .small(titleText: "", descriptionText: "")), @@ -166,13 +166,13 @@ final class NewTabPageMessagesModelTests: XCTestCase { sut.load() let model = try XCTUnwrap(sut.homeMessageViewModels.first) - model.onDidClose(.primaryAction(isShare: false)) + await model.onDidClose(.primaryAction(isShare: false)) XCTAssertEqual(PixelFiringMock.lastPixel, .remoteMessagePrimaryActionClicked) XCTAssertEqual(PixelFiringMock.lastParams, [PixelParameters.message: "foo"]) } - func testFiresPixelOnSecondaryAction() throws { + func testFiresPixelOnSecondaryAction() async throws { let sut = createSUT() messagesConfiguration.homeMessages = [ .mockRemote(withType: .small(titleText: "", descriptionText: "")), @@ -180,13 +180,13 @@ final class NewTabPageMessagesModelTests: XCTestCase { sut.load() let model = try XCTUnwrap(sut.homeMessageViewModels.first) - model.onDidClose(.secondaryAction(isShare: false)) + await model.onDidClose(.secondaryAction(isShare: false)) XCTAssertEqual(PixelFiringMock.lastPixel, .remoteMessageSecondaryActionClicked) XCTAssertEqual(PixelFiringMock.lastParams, [PixelParameters.message: "foo"]) } - func testDoesNotFirePixelOnCloseWhenMetricsAreDisabled() throws { + func testDoesNotFirePixelOnCloseWhenMetricsAreDisabled() async throws { let sut = createSUT() messagesConfiguration.homeMessages = [ .mockRemote(withType: .small(titleText: "", descriptionText: ""), isMetricsEnabled: false), @@ -195,13 +195,13 @@ final class NewTabPageMessagesModelTests: XCTestCase { let model = try XCTUnwrap(sut.homeMessageViewModels.first) - model.onDidClose(.close) + await model.onDidClose(.close) XCTAssertNil(PixelFiringMock.lastPixel) XCTAssertNil(PixelFiringMock.lastParams) } - func testDoesNotFirePixelOnActionWhenMetricsAreDisabled() throws { + func testDoesNotFirePixelOnActionWhenMetricsAreDisabled() async throws { let sut = createSUT() messagesConfiguration.homeMessages = [ .mockRemote(withType: .small(titleText: "", descriptionText: ""), isMetricsEnabled: false), @@ -209,13 +209,13 @@ final class NewTabPageMessagesModelTests: XCTestCase { sut.load() let model = try XCTUnwrap(sut.homeMessageViewModels.first) - model.onDidClose(.action(isShare: false)) + await model.onDidClose(.action(isShare: false)) XCTAssertNil(PixelFiringMock.lastPixel) XCTAssertNil(PixelFiringMock.lastParams) } - func testDoesNotFirePixelOnPrimaryActionWhenMetricsAreDisabled() throws { + func testDoesNotFirePixelOnPrimaryActionWhenMetricsAreDisabled() async throws { let sut = createSUT() messagesConfiguration.homeMessages = [ .mockRemote(withType: .small(titleText: "", descriptionText: ""), isMetricsEnabled: false), @@ -223,13 +223,13 @@ final class NewTabPageMessagesModelTests: XCTestCase { sut.load() let model = try XCTUnwrap(sut.homeMessageViewModels.first) - model.onDidClose(.primaryAction(isShare: false)) + await model.onDidClose(.primaryAction(isShare: false)) XCTAssertNil(PixelFiringMock.lastPixel) XCTAssertNil(PixelFiringMock.lastParams) } - func testDoesNotFirePixelOnSecondaryActionWhenMetricsAreDisabled() throws { + func testDoesNotFirePixelOnSecondaryActionWhenMetricsAreDisabled() async throws { let sut = createSUT() messagesConfiguration.homeMessages = [ .mockRemote(withType: .small(titleText: "", descriptionText: ""), isMetricsEnabled: false), @@ -237,7 +237,7 @@ final class NewTabPageMessagesModelTests: XCTestCase { sut.load() let model = try XCTUnwrap(sut.homeMessageViewModels.first) - model.onDidClose(.secondaryAction(isShare: false)) + await model.onDidClose(.secondaryAction(isShare: false)) XCTAssertNil(PixelFiringMock.lastPixel) XCTAssertNil(PixelFiringMock.lastParams)