From 211ab02e7c525dc24a048a39389b50a6ffb9d159 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 24 Jul 2024 10:48:04 +0200 Subject: [PATCH] Add unit tests and refactor RemoteMessagingStore --- .../CoreData/RemoteMessageUtils.swift | 47 +++++ .../RemoteMessagingStore.swift | 198 +++++++----------- .../RemoteMessagingStoreTests.swift | 159 +++++++++++++- 3 files changed, 276 insertions(+), 128 deletions(-) create mode 100644 Sources/RemoteMessaging/CoreData/RemoteMessageUtils.swift diff --git a/Sources/RemoteMessaging/CoreData/RemoteMessageUtils.swift b/Sources/RemoteMessaging/CoreData/RemoteMessageUtils.swift new file mode 100644 index 000000000..ad6b774d2 --- /dev/null +++ b/Sources/RemoteMessaging/CoreData/RemoteMessageUtils.swift @@ -0,0 +1,47 @@ +// +// RemoteMessageUtils.swift +// DuckDuckGo +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// 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 CoreData + +struct RemoteMessageUtils { + static func fetchRemoteMessage(with id: String, in context: NSManagedObjectContext) -> RemoteMessageManagedObject? { + let fetchRequest: NSFetchRequest = RemoteMessageManagedObject.fetchRequest() + fetchRequest.predicate = NSPredicate(format: "%K == %@", #keyPath(RemoteMessageManagedObject.id), id) + fetchRequest.fetchLimit = 1 + + return try? context.fetch(fetchRequest).first + } + + static func fetchScheduledRemoteMessage(in context: NSManagedObjectContext) -> RemoteMessageManagedObject? { + let fetchRequest: NSFetchRequest = RemoteMessageManagedObject.fetchRequest() + fetchRequest.predicate = NSPredicate( + format: "%K == %i", + #keyPath(RemoteMessageManagedObject.status), + RemoteMessagingStore.RemoteMessageStatus.scheduled.rawValue + ) + fetchRequest.fetchLimit = 1 + + return try? context.fetch(fetchRequest).first + } + + static func fetchAllRemoteMessages(in context: NSManagedObjectContext) -> [RemoteMessageManagedObject] { + let fetchRequest = RemoteMessageManagedObject.fetchRequest() + return (try? context.fetch(fetchRequest)) ?? [] + } +} diff --git a/Sources/RemoteMessaging/RemoteMessagingStore.swift b/Sources/RemoteMessaging/RemoteMessagingStore.swift index bc8a0050a..bc7964bcd 100644 --- a/Sources/RemoteMessaging/RemoteMessagingStore.swift +++ b/Sources/RemoteMessaging/RemoteMessagingStore.swift @@ -25,11 +25,8 @@ import Persistence public enum RemoteMessagingStoreError: Error { case saveConfigFailed - case invalidateConfigFailed case updateMessageShownFailed - case saveMessageFailed case updateMessageStatusFailed - case deleteScheduledMessageFailed } public final class RemoteMessagingStore: RemoteMessagingStoring { @@ -86,14 +83,23 @@ public final class RemoteMessagingStore: RemoteMessagingStoring { os_log("Remote messaging config - save processed version: %d", log: log, type: .debug, processorResult.version) let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType, name: Constants.privateContextName) - saveRemoteMessagingConfig(withVersion: processorResult.version, in: context) + context.performAndWait { + storeRemoteMessagingConfig(with: processorResult.version, in: context) - if let remoteMessage = processorResult.message { - addOrUpdate(remoteMessage: remoteMessage, in: context) - } else { - markScheduledMessagesAsDoneAndDeleteNeverShownMessages(in: context) + if let remoteMessage = processorResult.message { + addOrUpdate(remoteMessage: remoteMessage, in: context) + } else { + markScheduledMessagesAsDoneAndDeleteNeverShownMessages(in: context) + } + + do { + try context.save() + notificationCenter.post(name: Notifications.remoteMessagesDidChange, object: nil) + } catch { + errorEvents?.fire(.saveConfigFailed, error: error) + os_log("Failed to updare remote messages: %@", log: log, type: .error, error.localizedDescription) + } } - notificationCenter.post(name: Notifications.remoteMessagesDidChange, object: nil) } private func deleteScheduledMessagesIfNeeded() { @@ -103,9 +109,18 @@ public final class RemoteMessagingStore: RemoteMessagingStoring { let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType, name: Constants.privateContextName) - invalidateRemoteMessagingConfigs(in: context) - markScheduledMessagesAsDoneAndDeleteNeverShownMessages(in: context) - notificationCenter.post(name: Notifications.remoteMessagesDidChange, object: nil) + context.performAndWait { + invalidateRemoteMessagingConfigs(in: context) + markScheduledMessagesAsDoneAndDeleteNeverShownMessages(in: context) + + do { + try context.save() + notificationCenter.post(name: Notifications.remoteMessagesDidChange, object: nil) + } catch { + errorEvents?.fire(.saveConfigFailed, error: error) + os_log("Failed to updare remote messages: %@", log: log, type: .error, error.localizedDescription) + } + } } private let errorEvents: EventMapping? @@ -150,51 +165,28 @@ extension RemoteMessagingStore { extension RemoteMessagingStore { - private func saveRemoteMessagingConfig(withVersion version: Int64, in context: NSManagedObjectContext) { - context.performAndWait { - let fetchRequest: NSFetchRequest = RemoteMessagingConfigManagedObject.fetchRequest() - fetchRequest.fetchLimit = 1 - fetchRequest.predicate = NSPredicate(format: "version == %lld", version) - - if let results = try? context.fetch(fetchRequest), let result = results.first { - result.evaluationTimestamp = Date() - result.invalidate = false - } else { - let remoteMessagingConfigManagedObject = RemoteMessagingConfigManagedObject(context: context) - remoteMessagingConfigManagedObject.version = NSNumber(value: version) - remoteMessagingConfigManagedObject.evaluationTimestamp = Date() - remoteMessagingConfigManagedObject.invalidate = false - } + private func storeRemoteMessagingConfig(with version: Int64, in context: NSManagedObjectContext) { + let fetchRequest: NSFetchRequest = RemoteMessagingConfigManagedObject.fetchRequest() + fetchRequest.fetchLimit = 1 + fetchRequest.predicate = NSPredicate(format: "%K == %lld", #keyPath(RemoteMessagingConfigManagedObject.version), version) - do { - try context.save() - } catch { - errorEvents?.fire(.saveConfigFailed, error: error) - os_log("Failed to save remote messaging config: %@", - log: log, - type: .error, error.localizedDescription) - } + if let results = try? context.fetch(fetchRequest), let result = results.first { + result.evaluationTimestamp = Date() + result.invalidate = false + } else { + let remoteMessagingConfigManagedObject = RemoteMessagingConfigManagedObject(context: context) + remoteMessagingConfigManagedObject.version = NSNumber(value: version) + remoteMessagingConfigManagedObject.evaluationTimestamp = Date() + remoteMessagingConfigManagedObject.invalidate = false } } private func invalidateRemoteMessagingConfigs(in context: NSManagedObjectContext) { - context.performAndWait { - let fetchRequest: NSFetchRequest = RemoteMessagingConfigManagedObject.fetchRequest() - fetchRequest.returnsObjectsAsFaults = false - - guard let results = try? context.fetch(fetchRequest) else { return } + let fetchRequest: NSFetchRequest = RemoteMessagingConfigManagedObject.fetchRequest() + fetchRequest.returnsObjectsAsFaults = false - results.forEach { $0.invalidate = true } - - do { - try context.save() - } catch { - errorEvents?.fire(.invalidateConfigFailed, error: error) - os_log("Failed to save remote messaging config entity invalidate: %@", - log: log, - type: .error, error.localizedDescription) - } - } + guard let results = try? context.fetch(fetchRequest) else { return } + results.forEach { $0.invalidate = true } } } @@ -323,6 +315,13 @@ extension RemoteMessagingStore { context.performAndWait { updateRemoteMessage(withID: id, toStatus: .dismissed, in: context) invalidateRemoteMessagingConfigs(in: context) + + do { + try context.save() + } catch { + errorEvents?.fire(.updateMessageStatusFailed, error: error) + os_log("Error saving updateMessageStatus", log: log, type: .error) + } } } @@ -362,11 +361,11 @@ extension RemoteMessagingStore { #keyPath(RemoteMessageManagedObject.shown), !shown ) - guard let message = try? context.fetch(fetchRequest).first else { return } - - message.shown = shown - do { + guard let message = try context.fetch(fetchRequest).first else { + return + } + message.shown = shown try context.save() } catch { errorEvents?.fire(.updateMessageShownFailed, error: error) @@ -399,90 +398,41 @@ extension RemoteMessagingStore { // MARK: - RemoteMessageManagedObject Private Interface -struct RemoteMessageUtils { - static func fetchRemoteMessage(with id: String, in context: NSManagedObjectContext) -> RemoteMessageManagedObject? { - let fetchRequest: NSFetchRequest = RemoteMessageManagedObject.fetchRequest() - fetchRequest.predicate = NSPredicate(format: "%K == %@", #keyPath(RemoteMessageManagedObject.id), id) - fetchRequest.fetchLimit = 1 - - return try? context.fetch(fetchRequest).first - } - - static func fetchScheduledRemoteMessage(in context: NSManagedObjectContext) -> RemoteMessageManagedObject? { - let fetchRequest: NSFetchRequest = RemoteMessageManagedObject.fetchRequest() - fetchRequest.predicate = NSPredicate( - format: "%K == %i", - #keyPath(RemoteMessageManagedObject.status), - RemoteMessagingStore.RemoteMessageStatus.scheduled.rawValue - ) - fetchRequest.fetchLimit = 1 - - return try? context.fetch(fetchRequest).first - } -} - extension RemoteMessagingStore { private func addOrUpdate(remoteMessage: RemoteMessageModel, in context: NSManagedObjectContext) { - context.performAndWait { - if let scheduledRemoteMessage = RemoteMessageUtils.fetchScheduledRemoteMessage(in: context), scheduledRemoteMessage.id == remoteMessage.id { - scheduledRemoteMessage.message = RemoteMessageMapper.toString(remoteMessage) ?? "" - } else { - markScheduledMessagesAsDone(in: context) + if let scheduledRemoteMessage = RemoteMessageUtils.fetchScheduledRemoteMessage(in: context), scheduledRemoteMessage.id == remoteMessage.id { + scheduledRemoteMessage.message = RemoteMessageMapper.toString(remoteMessage) ?? "" + } else { + markScheduledMessagesAsDone(in: context) - let remoteMessageManagedObject = RemoteMessageUtils.fetchRemoteMessage(with: remoteMessage.id, in: context) - ?? RemoteMessageManagedObject(context: context) + let remoteMessageManagedObject = RemoteMessageUtils.fetchRemoteMessage(with: remoteMessage.id, in: context) + ?? RemoteMessageManagedObject(context: context) - remoteMessageManagedObject.message = RemoteMessageMapper.toString(remoteMessage) ?? "" - remoteMessageManagedObject.status = NSNumber(value: RemoteMessageStatus.scheduled.rawValue) + remoteMessageManagedObject.message = RemoteMessageMapper.toString(remoteMessage) ?? "" + remoteMessageManagedObject.status = NSNumber(value: RemoteMessageStatus.scheduled.rawValue) - if remoteMessageManagedObject.isInserted { - remoteMessageManagedObject.id = remoteMessage.id - remoteMessageManagedObject.shown = false - } - } - deleteDoneMessagesThatAreNotShown(in: context) - - do { - try context.save() - } catch { - errorEvents?.fire(.saveMessageFailed, error: error) - os_log("Failed to save remote message entity: %@", log: log, type: .error, error.localizedDescription) + if remoteMessageManagedObject.isInserted { + remoteMessageManagedObject.id = remoteMessage.id + remoteMessageManagedObject.shown = false } } + deleteNotShownDoneMessages(in: context) } private func markScheduledMessagesAsDoneAndDeleteNeverShownMessages(in context: NSManagedObjectContext) { - context.performAndWait { - markScheduledMessagesAsDone(in: context) - deleteDoneMessagesThatAreNotShown(in: context) - - do { - try context.save() - } catch { - errorEvents?.fire(.saveMessageFailed, error: error) - os_log("Failed to save remote message entity: %@", log: log, type: .error, error.localizedDescription) - } - } + markScheduledMessagesAsDone(in: context) + deleteNotShownDoneMessages(in: context) } private func updateRemoteMessage(withID id: String, toStatus status: RemoteMessageStatus, in context: NSManagedObjectContext) { - context.performAndWait { - let fetchRequest: NSFetchRequest = RemoteMessageManagedObject.fetchRequest() - fetchRequest.predicate = NSPredicate(format: "id == %@", id) - fetchRequest.returnsObjectsAsFaults = false - - guard let results = try? context.fetch(fetchRequest) else { return } + let fetchRequest: NSFetchRequest = RemoteMessageManagedObject.fetchRequest() + fetchRequest.predicate = NSPredicate(format: "id == %@", id) + fetchRequest.returnsObjectsAsFaults = false - results.forEach { $0.status = NSNumber(value: status.rawValue) } + guard let results = try? context.fetch(fetchRequest) else { return } - do { - try context.save() - } catch { - errorEvents?.fire(.updateMessageStatusFailed, error: error) - os_log("Error saving updateMessageStatus", log: log, type: .error) - } - } + results.forEach { $0.status = NSNumber(value: status.rawValue) } } private func markScheduledMessagesAsDone(in context: NSManagedObjectContext) { @@ -499,7 +449,7 @@ extension RemoteMessagingStore { } } - private func deleteDoneMessagesThatAreNotShown(in context: NSManagedObjectContext) { + private func deleteNotShownDoneMessages(in context: NSManagedObjectContext) { let fetchRequest: NSFetchRequest = RemoteMessageManagedObject.fetchRequest() fetchRequest.predicate = NSPredicate( format: "%K == %i AND %K == NO", diff --git a/Tests/RemoteMessagingTests/RemoteMessagingStoreTests.swift b/Tests/RemoteMessagingTests/RemoteMessagingStoreTests.swift index 8c1c13b8e..327d748dc 100644 --- a/Tests/RemoteMessagingTests/RemoteMessagingStoreTests.swift +++ b/Tests/RemoteMessagingTests/RemoteMessagingStoreTests.swift @@ -82,8 +82,8 @@ class RemoteMessagingStoreTests: XCTestCase { wait(for: [expectation], timeout: 10) } - func saveProcessedResultFetchRemoteMessage() throws -> RemoteMessageModel { - let processorResult = try processorResult() + func saveProcessedResultFetchRemoteMessage(for configJSON: String? = nil) throws -> RemoteMessageModel { + let processorResult = try processorResult(for: configJSON) // 1. saveProcessedResult() store.saveProcessedResult(processorResult) @@ -142,6 +142,124 @@ class RemoteMessagingStoreTests: XCTestCase { XCTAssertEqual(dismissedRemoteMessageIds.first, remoteMessage.id) } + func testConfigUpdateWhenMessageWasShownAndNotInteractedWithThenItIsNotRemovedFromDatabase() throws { + let remoteMessage = try saveProcessedResultFetchRemoteMessage(for: minimalConfig(version: 1, messageID: 1)) + store.updateRemoteMessage(withID: remoteMessage.id, asShown: true) + + let context = remoteMessagingDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + context.performAndWait { + let messages = RemoteMessageUtils.fetchAllRemoteMessages(in: context) + XCTAssertEqual(messages.count, 1) + let firstMessage = messages.first! + XCTAssertTrue(firstMessage.shown) + XCTAssertEqual(firstMessage.status?.int16Value, RemoteMessagingStore.RemoteMessageStatus.scheduled.rawValue) + } + + _ = try saveProcessedResultFetchRemoteMessage(for: minimalConfig(version: 2, messageID: 2)) + + context.performAndWait { + context.refreshAllObjects() + let messages = RemoteMessageUtils.fetchAllRemoteMessages(in: context) + XCTAssertEqual(messages.count, 2) + + let firstMessage = messages.first(where: { $0.id == "1" })! + XCTAssertTrue(firstMessage.shown) + XCTAssertEqual(firstMessage.status?.int16Value, RemoteMessagingStore.RemoteMessageStatus.done.rawValue) + + let secondMessage = messages.first(where: { $0.id == "2" })! + XCTAssertFalse(secondMessage.shown) + XCTAssertEqual(secondMessage.status?.int16Value, RemoteMessagingStore.RemoteMessageStatus.scheduled.rawValue) + } + } + + func testConfigUpdateWhenMessageWasInteractedWithThenItIsNotRemovedFromDatabase() throws { + let remoteMessage = try saveProcessedResultFetchRemoteMessage(for: minimalConfig(version: 1, messageID: 1)) + store.updateRemoteMessage(withID: remoteMessage.id, asShown: true) + store.dismissRemoteMessage(withID: remoteMessage.id) + + let context = remoteMessagingDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + context.performAndWait { + let messages = RemoteMessageUtils.fetchAllRemoteMessages(in: context) + XCTAssertEqual(messages.count, 1) + + let firstMessage = messages.first! + XCTAssertTrue(firstMessage.shown) + XCTAssertEqual(firstMessage.status?.int16Value, RemoteMessagingStore.RemoteMessageStatus.dismissed.rawValue) + } + + _ = try saveProcessedResultFetchRemoteMessage(for: minimalConfig(version: 2, messageID: 2)) + + context.performAndWait { + context.refreshAllObjects() + let messages = RemoteMessageUtils.fetchAllRemoteMessages(in: context) + XCTAssertEqual(messages.count, 2) + + let firstMessage = messages.first(where: { $0.id == "1" })! + XCTAssertTrue(firstMessage.shown) + XCTAssertEqual(firstMessage.status?.int16Value, RemoteMessagingStore.RemoteMessageStatus.dismissed.rawValue) + + let secondMessage = messages.first(where: { $0.id == "2" })! + XCTAssertFalse(secondMessage.shown) + XCTAssertEqual(secondMessage.status?.int16Value, RemoteMessagingStore.RemoteMessageStatus.scheduled.rawValue) + } + } + + func testConfigUpdateWhenMessageWasNotShownThenItIsRemovedFromDatabase() throws { + let remoteMessage = try saveProcessedResultFetchRemoteMessage(for: minimalConfig(version: 1, messageID: 1)) + + let context = remoteMessagingDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + context.performAndWait { + let messages = RemoteMessageUtils.fetchAllRemoteMessages(in: context) + XCTAssertEqual(messages.count, 1) + + let firstMessage = messages.first! + XCTAssertFalse(firstMessage.shown) + XCTAssertEqual(firstMessage.status?.int16Value, RemoteMessagingStore.RemoteMessageStatus.scheduled.rawValue) + } + + _ = try saveProcessedResultFetchRemoteMessage(for: minimalConfig(version: 2, messageID: 2)) + + context.performAndWait { + context.refreshAllObjects() + let messages = RemoteMessageUtils.fetchAllRemoteMessages(in: context) + XCTAssertEqual(messages.count, 1) + + let secondMessage = messages.first! + XCTAssertEqual(secondMessage.id, "2") + XCTAssertFalse(secondMessage.shown) + XCTAssertEqual(secondMessage.status?.int16Value, RemoteMessagingStore.RemoteMessageStatus.scheduled.rawValue) + } + } + + func testConfigUpdateWhenShownAndNotInteractedWithMessageIsReintroducedInNewConfigThenItIsMarkedAsScheduled() throws { + let remoteMessage = try saveProcessedResultFetchRemoteMessage(for: minimalConfig(version: 1, messageID: 1)) + store.updateRemoteMessage(withID: remoteMessage.id, asShown: true) + + let context = remoteMessagingDatabase.makeContext(concurrencyType: .privateQueueConcurrencyType) + context.performAndWait { + let messages = RemoteMessageUtils.fetchAllRemoteMessages(in: context) + XCTAssertEqual(messages.count, 1) + + let firstMessage = messages.first! + XCTAssertTrue(firstMessage.shown) + XCTAssertEqual(firstMessage.status?.int16Value, RemoteMessagingStore.RemoteMessageStatus.scheduled.rawValue) + } + + _ = try saveProcessedResultFetchRemoteMessage(for: minimalConfig(version: 2, messageID: 2)) + _ = try saveProcessedResultFetchRemoteMessage(for: minimalConfig(version: 3, messageID: 1)) + + context.performAndWait { + context.refreshAllObjects() + let messages = RemoteMessageUtils.fetchAllRemoteMessages(in: context) + XCTAssertEqual(messages.count, 1) + + let firstMessage = messages.first! + XCTAssertEqual(firstMessage.id, "1") + XCTAssertTrue(firstMessage.shown) // shown status is preserved + XCTAssertEqual(firstMessage.status?.int16Value, RemoteMessagingStore.RemoteMessageStatus.scheduled.rawValue) + } + } + // MARK: - Feature Flag func testWhenFeatureFlagIsDisabledThenScheduledRemoteMessagesAreDeleted() throws { @@ -274,8 +392,25 @@ class RemoteMessagingStoreTests: XCTestCase { return remoteMessagingConfig } - func processorResult() throws -> RemoteMessagingConfigProcessor.ProcessorResult { - let jsonRemoteMessagingConfig = try decodeJson(fileName: "remote-messaging-config-example.json") + func decodeJson(from jsonString: String) throws -> RemoteMessageResponse.JsonRemoteMessagingConfig { + let validJson = jsonString.data(using: .utf8)! + let remoteMessagingConfig = try JSONDecoder().decode(RemoteMessageResponse.JsonRemoteMessagingConfig.self, from: validJson) + XCTAssertNotNil(remoteMessagingConfig) + + return remoteMessagingConfig + } + + func processorResult(for configJSON: String? = nil) throws -> RemoteMessagingConfigProcessor.ProcessorResult { + let jsonRemoteMessagingConfig = try { + guard let configJSON else { + return try decodeJson(fileName: "remote-messaging-config-example.json") + } + return try decodeJson(from: configJSON) + }() + return try processorResult(for: jsonRemoteMessagingConfig) + } + + func processorResult(for jsonRemoteMessagingConfig: RemoteMessageResponse.JsonRemoteMessagingConfig) throws -> RemoteMessagingConfigProcessor.ProcessorResult { let remoteMessagingConfigMatcher = RemoteMessagingConfigMatcher( appAttributeMatcher: MobileAppAttributeMatcher(statisticsStore: MockStatisticsStore(), variantManager: MockVariantManager()), userAttributeMatcher: MobileUserAttributeMatcher( @@ -313,6 +448,22 @@ class RemoteMessagingStoreTests: XCTestCase { return RemoteMessagingConfigProcessor.ProcessorResult(version: 0, message: nil) } } + + func minimalConfig(version: Int, messageID: Int) -> String { + """ + { + "version": \(version), + "messages": [ + { + "id": "\(messageID)", + "content": { "messageType": "small", "titleText": "title", "descriptionText": "description" }, + "matchingRules": [], "exclusionRules": [] + } + ], + "rules": [] + } + """ + } } private final class MockRemoteMessagingSurveyActionMapper: RemoteMessagingSurveyActionMapping {