Core Data: Migrate storage usage for SystemStatusStore (#14559)

woocommerce · Dec 3, 2024 · 9bd4a47 · 9bd4a47
2 parents 0b7e751 + 1c434d8
commit 9bd4a47
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 48 deletions.
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
@@ -13,6 +13,7 @@
 - [Internal] Updated storage usage in CouponStore [https://github.com/woocommerce/woocommerce-ios/pull/14530]
 - [Internal] Update storage usage for BlazeStore [https://github.com/woocommerce/woocommerce-ios/pull/14532]
 - [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]
 - [Internal] Updated storage usage in SitePluginStore [https://github.com/woocommerce/woocommerce-ios/pull/14560]
 - [Internal] Updated storage usage in ShippingLabelStore [https://github.com/woocommerce/woocommerce-ios/pull/14566]
 - [*] Fixed: Improved the error message displayed when Bluetooth permission is denied during the card reader connection process. [https://github.com/woocommerce/woocommerce-ios/pull/14561]

diff --git a/Storage/Storage/Tools/StorageType+Extensions.swift b/Storage/Storage/Tools/StorageType+Extensions.swift
@@ -748,6 +748,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
@@ -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 1"
+        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: ["Plugin 1", "Plugin 4"])
+
+        // Then
+        XCTAssertEqual(storedSystemPlugins, [systemPlugin1, systemPlugin4])
+    }
+
     func test_loadSystemPlugin_by_siteID_and_name() throws {
         // Given
         let systemPlugin1 = storage.insertNewObject(ofType: SystemPlugin.self)

diff --git 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, Error>) -> 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)
     }
 }

diff --git 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 {

diff --git 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,10 @@ private extension SystemStatusStore {
     ///
     func upsertSystemPluginsInBackground(siteID: Int64,
                                          readonlySystemInformation: SystemStatus,
-                                         completionHandler: @escaping (Result<Void, Error>) -> 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.
@@ -117,10 +110,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)
@@ -138,13 +132,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) {