From c8889d7149ab7c351b63532c4b9b704f062cefda Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Wed, 10 Sep 2025 14:24:49 +0100 Subject: [PATCH 1/4] Delete data from all grdb tables on logout --- .../Sources/Storage/GRDB/GRDBManager.swift | 26 ++++ .../StorageTests/GRDB/GRDBManagerTests.swift | 140 ++++++++++++++++++ .../Yosemite/DefaultStoresManager.swift | 5 + 3 files changed, 171 insertions(+) diff --git a/Modules/Sources/Storage/GRDB/GRDBManager.swift b/Modules/Sources/Storage/GRDB/GRDBManager.swift index bbe9eea76c7..cfc503c101f 100644 --- a/Modules/Sources/Storage/GRDB/GRDBManager.swift +++ b/Modules/Sources/Storage/GRDB/GRDBManager.swift @@ -3,6 +3,7 @@ import GRDB public protocol GRDBManagerProtocol { var databaseConnection: GRDBDatabaseConnection { get } + func reset() throws } public protocol GRDBDatabaseConnection: DatabaseReader & DatabaseWriter {} @@ -27,6 +28,31 @@ public final class GRDBManager: GRDBManagerProtocol { self.databaseConnection = try DatabaseQueue() try migrateIfNeeded() } + + /// Resets the database by deleting all data from all tables + /// Used when user logs out to ensure no data leaks between sessions + public func reset() throws { + try databaseConnection.write { db in + // Disable foreign key constraints temporarily to avoid dependency issues + try db.execute(sql: "PRAGMA foreign_keys = OFF") + + // Get all user tables (excluding sqlite internal tables) + let tableNames = try String.fetchAll(db, sql: """ + SELECT name FROM sqlite_master + WHERE type = 'table' + AND name NOT LIKE 'sqlite_%' + AND name NOT LIKE 'grdb_%' + """) + + // Delete all data from each table + for tableName in tableNames { + try db.execute(sql: "DELETE FROM \(tableName)") + } + + // Re-enable foreign key constraints + try db.execute(sql: "PRAGMA foreign_keys = ON") + } + } } private extension GRDBManager { diff --git a/Modules/Tests/StorageTests/GRDB/GRDBManagerTests.swift b/Modules/Tests/StorageTests/GRDB/GRDBManagerTests.swift index 3b4ad57ff81..dc58e7c2b33 100644 --- a/Modules/Tests/StorageTests/GRDB/GRDBManagerTests.swift +++ b/Modules/Tests/StorageTests/GRDB/GRDBManagerTests.swift @@ -461,6 +461,146 @@ struct GRDBManagerTests { #expect(variations.allSatisfy { $0.siteID == sampleSiteID }) #expect(variations.allSatisfy { $0.productID == 100 }) } + } + + struct ResetTests { + let manager: GRDBManager + let sampleSiteID: Int64 = 1 + + init() throws { + self.manager = try GRDBManager() + try manager.databaseConnection.write { db in + let record = TestSite(id: sampleSiteID) + try record.insert(db) + } + } + + @Test("Reset clears all data from database") + func test_reset_clears_all_data_from_database() throws { + // Given - Insert comprehensive test data + try manager.databaseConnection.write { db in + // Insert second site for more comprehensive test + let site2 = TestSite(id: 2) + try site2.insert(db) + + // Insert products for both sites + for siteID in [sampleSiteID, Int64(2)] { + for i in 1...2 { + let product = TestProduct( + siteID: siteID, + id: Int64(i + (siteID == 1 ? 0 : 10)), + name: "Product \(i) Site \(siteID)", + productTypeKey: "variable", + price: "\(i * 10).00", + downloadable: false, + parentID: 0, + manageStock: false, + stockStatusKey: "" + ) + try product.insert(db) + + // Insert variations + let variation = TestProductVariation( + siteID: siteID, + id: Int64(200 + i + (siteID == 1 ? 0 : 10)), + productID: product.id, + price: "\(i * 12).00", + downloadable: false, + manageStock: false, + stockStatusKey: "" + ) + try variation.insert(db) + + // Insert product attributes + let productAttribute = TestProductAttribute( + productID: product.id, + name: "Color \(i)", + position: i, + visible: true, + variation: true, + options: ["Red", "Blue", "Green"] + ) + try productAttribute.insert(db) + + // Insert variation attributes + let variationAttribute = TestProductVariationAttribute( + productVariationID: variation.id, + name: "Size \(i)", + option: "Large" + ) + try variationAttribute.insert(db) + } + } + } + + // Verify data exists before reset + let countsBefore = try manager.databaseConnection.read { db in + return ( + sites: try TestSite.fetchCount(db), + products: try TestProduct.fetchCount(db), + variations: try TestProductVariation.fetchCount(db), + productAttributes: try TestProductAttribute.fetchCount(db), + variationAttributes: try TestProductVariationAttribute.fetchCount(db) + ) + } + + #expect(countsBefore.sites == 2) // Original site + new site + #expect(countsBefore.products == 4) + #expect(countsBefore.variations == 4) + #expect(countsBefore.productAttributes == 4) + #expect(countsBefore.variationAttributes == 4) + + // When - Reset database + try manager.reset() + + // Then - All data should be cleared + let countsAfter = try manager.databaseConnection.read { db in + return ( + sites: try TestSite.fetchCount(db), + products: try TestProduct.fetchCount(db), + variations: try TestProductVariation.fetchCount(db), + productAttributes: try TestProductAttribute.fetchCount(db), + variationAttributes: try TestProductVariationAttribute.fetchCount(db) + ) + } + + #expect(countsAfter.sites == 0) + #expect(countsAfter.products == 0) + #expect(countsAfter.variations == 0) + #expect(countsAfter.productAttributes == 0) + #expect(countsAfter.variationAttributes == 0) + } + + @Test("Reset can be called multiple times without error") + func test_reset_can_be_called_multiple_times() throws { + // Given - Some test data + try manager.databaseConnection.write { db in + let product = TestProduct( + siteID: sampleSiteID, + id: 100, + name: "Test Product", + productTypeKey: "simple", + price: "10.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockStatusKey: "" + ) + try product.insert(db) + } + + // When - Reset multiple times + try manager.reset() + try manager.reset() + try manager.reset() + + // Then - Should not throw and database should be empty + let productCount = try manager.databaseConnection.read { db in + try TestProduct.fetchCount(db) + } + + #expect(productCount == 0) + } @Test("Cannot insert product without valid site") func test_cannot_insert_product_without_valid_site() throws { diff --git a/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift b/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift index 8e631a36b32..601a415de0b 100644 --- a/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift +++ b/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift @@ -269,6 +269,11 @@ class DefaultStoresManager: StoresManager { ServiceLocator.analytics.refreshUserData() ZendeskProvider.shared.reset() ServiceLocator.storageManager.reset() + do { + try ServiceLocator.grdbManager.reset() + } catch { + DDLogError("Could not reset GRDB database: \(error)") + } ServiceLocator.productImageUploader.reset() updateAndReloadWidgetInformation(with: nil) From ca9f13cbd5c095202b51ac1c801bf3d9e209ea28 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Wed, 10 Sep 2025 15:05:05 +0100 Subject: [PATCH 2/4] Fix test structur --- .../StorageTests/GRDB/GRDBManagerTests.swift | 80 +++++++++---------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/Modules/Tests/StorageTests/GRDB/GRDBManagerTests.swift b/Modules/Tests/StorageTests/GRDB/GRDBManagerTests.swift index dc58e7c2b33..4a65a0ae150 100644 --- a/Modules/Tests/StorageTests/GRDB/GRDBManagerTests.swift +++ b/Modules/Tests/StorageTests/GRDB/GRDBManagerTests.swift @@ -461,6 +461,46 @@ struct GRDBManagerTests { #expect(variations.allSatisfy { $0.siteID == sampleSiteID }) #expect(variations.allSatisfy { $0.productID == 100 }) } + + @Test("Cannot insert product without valid site") + func test_cannot_insert_product_without_valid_site() throws { + // When/Then + #expect(throws: DatabaseError.self) { + try manager.databaseConnection.write { db in + let product = TestProduct( + siteID: 999, // Non-existent site + id: 100, + name: "Orphaned Product", + productTypeKey: "simple", + price: "10.00", + downloadable: false, + parentID: 0, + manageStock: false, + stockStatusKey: "" + ) + try product.insert(db) + } + } + } + + @Test("Cannot insert variation without valid product") + func test_cannot_insert_variation_without_valid_product() throws { + // When/Then + #expect(throws: DatabaseError.self) { + try manager.databaseConnection.write { db in + let variation = TestProductVariation( + siteID: sampleSiteID, + id: 200, + productID: 999, // Non-existent product + price: "12.00", + downloadable: false, + manageStock: false, + stockStatusKey: "" + ) + try variation.insert(db) + } + } + } } struct ResetTests { @@ -601,46 +641,6 @@ struct GRDBManagerTests { #expect(productCount == 0) } - - @Test("Cannot insert product without valid site") - func test_cannot_insert_product_without_valid_site() throws { - // When/Then - #expect(throws: DatabaseError.self) { - try manager.databaseConnection.write { db in - let product = TestProduct( - siteID: 999, // Non-existent site - id: 100, - name: "Orphaned Product", - productTypeKey: "simple", - price: "10.00", - downloadable: false, - parentID: 0, - manageStock: false, - stockStatusKey: "" - ) - try product.insert(db) - } - } - } - - @Test("Cannot insert variation without valid product") - func test_cannot_insert_variation_without_valid_product() throws { - // When/Then - #expect(throws: DatabaseError.self) { - try manager.databaseConnection.write { db in - let variation = TestProductVariation( - siteID: sampleSiteID, - id: 200, - productID: 999, // Non-existent product - price: "12.00", - downloadable: false, - manageStock: false, - stockStatusKey: "" - ) - try variation.insert(db) - } - } - } } } From 0ee71db167f93147de4dbe2872ea81d0a114b880 Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Thu, 11 Sep 2025 09:38:32 +0800 Subject: [PATCH 3/4] Override `deauthenticate` in `WooCommerceTests.MockStoresManager` so that it does not call `DefaultStoresManager.deauthenticate` which accesses ServiceLocator dependencies. --- WooCommerce/WooCommerceTests/Mocks/MockStoresManager.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/WooCommerce/WooCommerceTests/Mocks/MockStoresManager.swift b/WooCommerce/WooCommerceTests/Mocks/MockStoresManager.swift index 44b0beab055..4bfb41b832b 100644 --- a/WooCommerce/WooCommerceTests/Mocks/MockStoresManager.swift +++ b/WooCommerce/WooCommerceTests/Mocks/MockStoresManager.swift @@ -50,6 +50,10 @@ final class MockStoresManager: DefaultStoresManager { func reset() { receivedActions = [] } + + override func deauthenticate() -> any StoresManager { + self + } } // MARK: - Mocking From e931cbe99d14ede86248d234f5d753165a35f1f2 Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Thu, 11 Sep 2025 09:51:26 +0800 Subject: [PATCH 4/4] Fix crash from `ServiceLocator.grdbManager` when feature flag is disabled by only resetting `ServiceLocator.grdbManager` when feature flag is enabled. `MockStoresManager.deauthenticate` depends on `DefaultStoresManager` implementation for some auth related test cases. --- .../Classes/Yosemite/DefaultStoresManager.swift | 12 ++++++++---- .../WooCommerceTests/Mocks/MockStoresManager.swift | 4 ---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift b/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift index 601a415de0b..ebfc51eed6f 100644 --- a/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift +++ b/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift @@ -269,11 +269,15 @@ class DefaultStoresManager: StoresManager { ServiceLocator.analytics.refreshUserData() ZendeskProvider.shared.reset() ServiceLocator.storageManager.reset() - do { - try ServiceLocator.grdbManager.reset() - } catch { - DDLogError("Could not reset GRDB database: \(error)") + + if ServiceLocator.featureFlagService.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1) { + do { + try ServiceLocator.grdbManager.reset() + } catch { + DDLogError("Could not reset GRDB database: \(error)") + } } + ServiceLocator.productImageUploader.reset() updateAndReloadWidgetInformation(with: nil) diff --git a/WooCommerce/WooCommerceTests/Mocks/MockStoresManager.swift b/WooCommerce/WooCommerceTests/Mocks/MockStoresManager.swift index 4bfb41b832b..44b0beab055 100644 --- a/WooCommerce/WooCommerceTests/Mocks/MockStoresManager.swift +++ b/WooCommerce/WooCommerceTests/Mocks/MockStoresManager.swift @@ -50,10 +50,6 @@ final class MockStoresManager: DefaultStoresManager { func reset() { receivedActions = [] } - - override func deauthenticate() -> any StoresManager { - self - } } // MARK: - Mocking