From 57cc0db71c41a9faa4a3fe5d520cff292ead6843 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Fri, 29 Nov 2024 14:57:48 +0700 Subject: [PATCH 1/8] Use performAndSave to upsert system plugins --- .../Yosemite/Stores/SystemStatusStore.swift | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/Yosemite/Yosemite/Stores/SystemStatusStore.swift b/Yosemite/Yosemite/Stores/SystemStatusStore.swift index 396a84dc1a7..8f23e88a183 100644 --- a/Yosemite/Yosemite/Stores/SystemStatusStore.swift +++ b/Yosemite/Yosemite/Stores/SystemStatusStore.swift @@ -52,7 +52,7 @@ private extension SystemStatusStore { switch result { case .success(let systemInformation): self.updateStoreID(siteID: siteID, readonlySystemInformation: systemInformation) - self.upsertSystemPluginsInBackground(siteID: siteID, readonlySystemInformation: systemInformation) { [weak self] _ in + self.upsertSystemPluginsInBackground(siteID: siteID, readonlySystemInformation: systemInformation) { [weak self] in guard let self else { return } let systemPlugins = self.storageManager.viewStorage.loadSystemPlugins(siteID: siteID).map { $0.toReadOnly() } completionHandler(.success(.init(storeID: systemInformation.environment?.storeID, systemPlugins: systemPlugins))) @@ -77,17 +77,11 @@ private extension SystemStatusStore { /// func upsertSystemPluginsInBackground(siteID: Int64, readonlySystemInformation: SystemStatus, - completionHandler: @escaping (Result) -> Void) { - let writerStorage = storageManager.writerDerivedStorage - writerStorage.perform { - self.upsertSystemPlugins(siteID: siteID, readonlySystemInformation: readonlySystemInformation, in: writerStorage) - } - - storageManager.saveDerivedType(derivedStorage: writerStorage) { - DispatchQueue.main.async { - completionHandler(.success(())) - } - } + completionHandler: @escaping () -> Void) { + storageManager.performAndSave({ [weak self] storage in + self?.upsertSystemPlugins(siteID: siteID, readonlySystemInformation: readonlySystemInformation, in: storage) + }, completion: completionHandler, on: .main) + } /// Updates the store id from the system information. From 26c12930f78ccc638f80c903c84b524cf8c1f5b1 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Fri, 29 Nov 2024 15:09:39 +0700 Subject: [PATCH 2/8] Add new helper method to fetch system plugins by siteID and names --- .../Tools/StorageType+Extensions.swift | 8 +++++ .../Tools/StorageTypeExtensionsTests.swift | 29 +++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/Storage/Storage/Tools/StorageType+Extensions.swift b/Storage/Storage/Tools/StorageType+Extensions.swift index e36ab0a091d..82bb231db90 100644 --- a/Storage/Storage/Tools/StorageType+Extensions.swift +++ b/Storage/Storage/Tools/StorageType+Extensions.swift @@ -741,6 +741,14 @@ public extension StorageType { return allObjects(ofType: SystemPlugin.self, matching: predicate, sortedBy: [descriptor]) } + /// Returns stored system plugins for a provided `siteID` matching the given `names` + /// + func loadSystemPlugins(siteID: Int64, matching names: [String]) -> [SystemPlugin] { + let predicate = NSPredicate(format: "siteID == %lld && name in %@", siteID, names) + let descriptor = NSSortDescriptor(keyPath: \SystemPlugin.name, ascending: true) + return allObjects(ofType: SystemPlugin.self, matching: predicate, sortedBy: [descriptor]) + } + /// Returns a system plugin with a specified `siteID` and `name` /// func loadSystemPlugin(siteID: Int64, name: String) -> SystemPlugin? { diff --git a/Storage/StorageTests/Tools/StorageTypeExtensionsTests.swift b/Storage/StorageTests/Tools/StorageTypeExtensionsTests.swift index da603c5cc02..67d5088eb35 100644 --- a/Storage/StorageTests/Tools/StorageTypeExtensionsTests.swift +++ b/Storage/StorageTests/Tools/StorageTypeExtensionsTests.swift @@ -1344,7 +1344,7 @@ final class StorageTypeExtensionsTests: XCTestCase { // MARK: - System plugins - func test_loadSystemPlugins_by_siteID_and_sorted_by_name() throws { + func test_loadSystemPlugins_by_siteID_and_sorted_by_name() { // Given let systemPlugin1 = storage.insertNewObject(ofType: SystemPlugin.self) systemPlugin1.name = "Plugin 1" @@ -1359,12 +1359,37 @@ final class StorageTypeExtensionsTests: XCTestCase { systemPlugin3.siteID = sampleSiteID // When - let storedSystemPlugins = try XCTUnwrap(storage.loadSystemPlugins(siteID: sampleSiteID)) + let storedSystemPlugins = storage.loadSystemPlugins(siteID: sampleSiteID) // Then XCTAssertEqual(storedSystemPlugins, [systemPlugin1, systemPlugin3]) } + func test_loadSystemPlugins_by_siteID_matching_names() { + // Given + let systemPlugin1 = storage.insertNewObject(ofType: SystemPlugin.self) + systemPlugin1.name = "Plugin 1" + systemPlugin1.siteID = sampleSiteID + + let systemPlugin2 = storage.insertNewObject(ofType: SystemPlugin.self) + systemPlugin2.name = "Plugin 2" + systemPlugin2.siteID = sampleSiteID + 1 + + let systemPlugin3 = storage.insertNewObject(ofType: SystemPlugin.self) + systemPlugin3.name = "Plugin 3" + systemPlugin3.siteID = sampleSiteID + + let systemPlugin4 = storage.insertNewObject(ofType: SystemPlugin.self) + systemPlugin4.name = "Plugin 4" + systemPlugin4.siteID = sampleSiteID + + // When + let storedSystemPlugins = storage.loadSystemPlugins(siteID: sampleSiteID, matching: ["Plugin1", "Plugin4"]) + + // Then + XCTAssertEqual(Set(storedSystemPlugins), Set([systemPlugin1, systemPlugin4])) + } + func test_loadSystemPlugin_by_siteID_and_name() throws { // Given let systemPlugin1 = storage.insertNewObject(ofType: SystemPlugin.self) From 7e3fdf190deeb9a1f133ea5b8bca4885294141a6 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Fri, 29 Nov 2024 15:09:52 +0700 Subject: [PATCH 3/8] Optimize fetch requests for system plugins --- Yosemite/Yosemite/Stores/SystemStatusStore.swift | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/Yosemite/Yosemite/Stores/SystemStatusStore.swift b/Yosemite/Yosemite/Stores/SystemStatusStore.swift index 8f23e88a183..a6609f8c04a 100644 --- a/Yosemite/Yosemite/Stores/SystemStatusStore.swift +++ b/Yosemite/Yosemite/Stores/SystemStatusStore.swift @@ -111,10 +111,11 @@ private extension SystemStatusStore { return activePlugins + inactivePlugins }() + let storedPlugins = storage.loadSystemPlugins(siteID: siteID, matching: readonlySystemPlugins.map { $0.name }) readonlySystemPlugins.forEach { readonlySystemPlugin in // load or create new StorageSystemPlugin matching the readonly one let storageSystemPlugin: StorageSystemPlugin = { - if let systemPlugin = storage.loadSystemPlugin(siteID: readonlySystemPlugin.siteID, name: readonlySystemPlugin.name) { + if let systemPlugin = storedPlugins.first(where: { $0.name == readonlySystemPlugin.name }) { return systemPlugin } return storage.insertNewObject(ofType: StorageSystemPlugin.self) @@ -132,13 +133,9 @@ private extension SystemStatusStore { /// Useful when a plugin has had multiple names. /// func fetchSystemPlugin(siteID: Int64, systemPluginNameList: [String], completionHandler: @escaping (SystemPlugin?) -> Void) { - let viewStorage = storageManager.viewStorage - for systemPluginName in systemPluginNameList { - if let systemPlugin = viewStorage.loadSystemPlugin(siteID: siteID, name: systemPluginName)?.toReadOnly() { - return completionHandler(systemPlugin) - } - } - completionHandler(nil) + let matchingPlugins = storageManager.viewStorage.loadSystemPlugins(siteID: siteID, matching: systemPluginNameList) + .map { $0.toReadOnly() } + completionHandler(matchingPlugins.first) } func fetchSystemPluginWithPath(siteID: Int64, pluginPath: String, onCompletion: @escaping (SystemPlugin?) -> Void) { From 87793ec1b481299e62bf303497786af83ad67abe Mon Sep 17 00:00:00 2001 From: Huong Do Date: Fri, 29 Nov 2024 15:16:14 +0700 Subject: [PATCH 4/8] Update unit tests --- Storage/StorageTests/Tools/StorageTypeExtensionsTests.swift | 4 ++-- Yosemite/Yosemite/Stores/SystemStatusStore.swift | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Storage/StorageTests/Tools/StorageTypeExtensionsTests.swift b/Storage/StorageTests/Tools/StorageTypeExtensionsTests.swift index 67d5088eb35..04cfcf55e77 100644 --- a/Storage/StorageTests/Tools/StorageTypeExtensionsTests.swift +++ b/Storage/StorageTests/Tools/StorageTypeExtensionsTests.swift @@ -1384,10 +1384,10 @@ final class StorageTypeExtensionsTests: XCTestCase { systemPlugin4.siteID = sampleSiteID // When - let storedSystemPlugins = storage.loadSystemPlugins(siteID: sampleSiteID, matching: ["Plugin1", "Plugin4"]) + let storedSystemPlugins = storage.loadSystemPlugins(siteID: sampleSiteID, matching: ["Plugin 1", "Plugin 4"]) // Then - XCTAssertEqual(Set(storedSystemPlugins), Set([systemPlugin1, systemPlugin4])) + XCTAssertEqual(storedSystemPlugins, [systemPlugin1, systemPlugin4]) } func test_loadSystemPlugin_by_siteID_and_name() throws { diff --git a/Yosemite/Yosemite/Stores/SystemStatusStore.swift b/Yosemite/Yosemite/Stores/SystemStatusStore.swift index a6609f8c04a..31a1248f74f 100644 --- a/Yosemite/Yosemite/Stores/SystemStatusStore.swift +++ b/Yosemite/Yosemite/Stores/SystemStatusStore.swift @@ -81,7 +81,6 @@ private extension SystemStatusStore { storageManager.performAndSave({ [weak self] storage in self?.upsertSystemPlugins(siteID: siteID, readonlySystemInformation: readonlySystemInformation, in: storage) }, completion: completionHandler, on: .main) - } /// Updates the store id from the system information. From 4d544d7747b5577be3fe6f0912e8f245909cfbe7 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Fri, 29 Nov 2024 15:39:22 +0700 Subject: [PATCH 5/8] Update release notes --- RELEASE-NOTES.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index e0aa9463ffb..d3dc018eab1 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -7,6 +7,7 @@ - [**] Media Library: On sites logged in with application password, when picking image from WordPress Media Library, all images will now load correctly. [https://github.com/woocommerce/woocommerce-ios/pull/14444] - [Internal] Updated storage usage in CouponStore [https://github.com/woocommerce/woocommerce-ios/pull/14530] - [Internal] Updated storage usage in ProductShippingClassStore [https://github.com/woocommerce/woocommerce-ios/pull/14520] +- [Internal] Updated storage usage in SystemStatusStore [https://github.com/woocommerce/woocommerce-ios/pull/14559] 21.2 ----- From 8487cf0d2315bc0c59c0dfd8b1ec8dea04951003 Mon Sep 17 00:00:00 2001 From: Huong Do Date: Fri, 29 Nov 2024 16:44:31 +0700 Subject: [PATCH 6/8] Sync system plugin in plugin list --- .../Dashboard/Settings/Plugins/PluginListViewModel.swift | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/Plugins/PluginListViewModel.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/Plugins/PluginListViewModel.swift index 5be123e1a2b..059ade8ee44 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Settings/Plugins/PluginListViewModel.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Settings/Plugins/PluginListViewModel.swift @@ -59,7 +59,14 @@ final class PluginListViewModel { /// Manually sync plugins. /// func syncPlugins(onCompletion: @escaping (Result) -> Void) { - let action = SitePluginAction.synchronizeSitePlugins(siteID: siteID, onCompletion: onCompletion) + let action = SystemStatusAction.synchronizeSystemInformation(siteID: siteID, onCompletion: { result in + switch result { + case .success: + onCompletion(.success(())) + case .failure(let error): + onCompletion(.failure(error)) + } + }) storesManager.dispatch(action) } } From 6ec6eb757ce376b94d7929b4036f11a687157b2c Mon Sep 17 00:00:00 2001 From: Huong Do Date: Fri, 29 Nov 2024 17:24:38 +0700 Subject: [PATCH 7/8] Update unit tests for PluginListViewModel --- .../Plugins/PluginListViewModelTests.swift | 32 ++++--------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Plugins/PluginListViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Plugins/PluginListViewModelTests.swift index 40d596216b8..8c4569fd6ab 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Plugins/PluginListViewModelTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Plugins/PluginListViewModelTests.swift @@ -31,9 +31,9 @@ class PluginListViewModelTests: XCTestCase { // Given let storesManager = MockStoresManager(sessionManager: .testingInstance) var triggeredSiteID: Int64? - storesManager.whenReceivingAction(ofType: SitePluginAction.self) { action in + storesManager.whenReceivingAction(ofType: SystemStatusAction.self) { action in switch action { - case .synchronizeSitePlugins(let siteID, _): + case .synchronizeSystemInformation(let siteID, _): triggeredSiteID = siteID default: break @@ -51,10 +51,10 @@ class PluginListViewModelTests: XCTestCase { func test_syncPlugins_returns_success_when_synchronizeSitePlugins_action_completes_successfully() { // Given let storesManager = MockStoresManager(sessionManager: .testingInstance) - storesManager.whenReceivingAction(ofType: SitePluginAction.self) { action in + storesManager.whenReceivingAction(ofType: SystemStatusAction.self) { action in switch action { - case .synchronizeSitePlugins(_, let completion): - completion(.success(())) + case .synchronizeSystemInformation(_, let completion): + completion(.success(.fake())) default: break } @@ -75,9 +75,9 @@ class PluginListViewModelTests: XCTestCase { func test_syncPlugins_returns_error_when_synchronizeSitePlugins_action_fails() { // Given let storesManager = MockStoresManager(sessionManager: .testingInstance) - storesManager.whenReceivingAction(ofType: SitePluginAction.self) { action in + storesManager.whenReceivingAction(ofType: SystemStatusAction.self) { action in switch action { - case .synchronizeSitePlugins(_, let completion): + case .synchronizeSystemInformation(_, let completion): completion(.failure(MockPluginError.mockError)) default: break @@ -97,24 +97,6 @@ class PluginListViewModelTests: XCTestCase { } } -// MARK: - Storage helpers -// -private extension PluginListViewModelTests { - func insert(_ readOnlyPlugin: Yosemite.SitePlugin) { - let plugin = storage.insertNewObject(ofType: StorageSitePlugin.self) - plugin.update(with: readOnlyPlugin) - storage.saveIfNeeded() - } - - func updateStorage(with readOnlyPlugin: Yosemite.SitePlugin) { - guard let plugin = storage.loadPlugin(siteID: readOnlyPlugin.siteID, name: readOnlyPlugin.name) else { - return - } - plugin.update(with: readOnlyPlugin) - storage.saveIfNeeded() - } -} - // MARK: - Mock types // private extension PluginListViewModelTests { From 51eeb2e35b8bf2935d499f3417166b16579b9c7c Mon Sep 17 00:00:00 2001 From: Huong Do Date: Tue, 3 Dec 2024 11:59:25 +0700 Subject: [PATCH 8/8] Update unit test to verify plugins of the same name in different sites --- Storage/StorageTests/Tools/StorageTypeExtensionsTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Storage/StorageTests/Tools/StorageTypeExtensionsTests.swift b/Storage/StorageTests/Tools/StorageTypeExtensionsTests.swift index 1bda85e57d8..dbab87bc5c5 100644 --- a/Storage/StorageTests/Tools/StorageTypeExtensionsTests.swift +++ b/Storage/StorageTests/Tools/StorageTypeExtensionsTests.swift @@ -1372,7 +1372,7 @@ final class StorageTypeExtensionsTests: XCTestCase { systemPlugin1.siteID = sampleSiteID let systemPlugin2 = storage.insertNewObject(ofType: SystemPlugin.self) - systemPlugin2.name = "Plugin 2" + systemPlugin2.name = "Plugin 1" systemPlugin2.siteID = sampleSiteID + 1 let systemPlugin3 = storage.insertNewObject(ofType: SystemPlugin.self)