diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Storage/DataBrokerProtectionDatabaseMigrations.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Storage/DataBrokerProtectionDatabaseMigrations.swift index 4c41888d1d..782cf28bae 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Storage/DataBrokerProtectionDatabaseMigrations.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Storage/DataBrokerProtectionDatabaseMigrations.swift @@ -220,10 +220,14 @@ final class DataBrokerProtectionDatabaseMigrations { try deleteOrphanedRecords(database: database) // Recreate tables to add correct foreign key constraints try recreateTablesV3(database: database) - /* - Ideas - Run violation check then... - - Run deletion again if explicit violation check fails - */ + + // As a precaution, re-run orphan deletion if necessary + do { + // Throws an error if a foreign key violation exists in the database. + try database.checkForeignKeys() + } catch { + try deleteOrphanedRecords(database: database) + } } private static func deleteOrphanedRecords(database: Database) throws { diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Storage/DataBrokerProtectionDatabaseProvider.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Storage/DataBrokerProtectionDatabaseProvider.swift index 2767e22128..c4af9c74da 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Storage/DataBrokerProtectionDatabaseProvider.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Storage/DataBrokerProtectionDatabaseProvider.swift @@ -75,8 +75,14 @@ protocol DataBrokerProtectionDatabaseProvider: SecureStorageDatabaseProvider { func fetchAttemptInformation(for extractedProfileId: Int64) throws -> OptOutAttemptDB? func save(_ optOutAttemptDB: OptOutAttemptDB) throws - // Test Helper Methods + // MARK: Debug & Test Helper Methods + + /// Dumps the database contents to a file specified by the provided URL + /// - Parameter url: URL of file to write to func dumpDatabase(to url: URL) throws + + /// Restores a database from a file + /// - Parameter url: URL of the source file func restoreDatabase(from url: URL) throws } @@ -560,6 +566,14 @@ final class DefaultDataBrokerProtectionDatabaseProvider: GRDBSecureStorageDataba } extension DatabaseValue { + + /// Returns the SQL representation of the `DatabaseValue`. + /// + /// This property converts the database value to a string that can be used in an SQL statement. + /// The conversion handles different storage types, ensuring that the value is properly formatted + /// and escaped for SQL. + /// + /// - Returns: A `String` representing the SQL expression of the `DatabaseValue`. var sqlExpression: String { switch storage { case .null: @@ -577,6 +591,12 @@ extension DatabaseValue { } extension Data { + + /// Converts `Data` to a hexadecimal string representation. + /// + /// This extension is used to format data so it can be inserted into SQL statements. + /// + /// - Returns: A `String` representing the hexadecimal encoding of the data. func hexEncodedString() -> String { return map { String(format: "%02hhx", $0) }.joined() } diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionDatabaseProviderTests.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionDatabaseProviderTests.swift index 1d5e4401a1..58d3ae611e 100644 --- a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionDatabaseProviderTests.swift +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionDatabaseProviderTests.swift @@ -48,21 +48,7 @@ final class DataBrokerProtectionDatabaseProviderTests: XCTestCase { } } - func testV3MigrationCleansUpOrphanedRecords() throws { - // Given - let failingMigration: (inout DatabaseMigrator) throws -> Void = { migrator in - migrator.registerMigration("v3") { database in - // This failing migration is used to ensure the database contains violations - try database.checkForeignKeys() - } - } - XCTAssertThrowsError(try DefaultDataBrokerProtectionDatabaseProvider(file: vaultURL, key: key, registerMigrationsHandler: failingMigration)) - - // When - Then - XCTAssertNoThrow(try DefaultDataBrokerProtectionDatabaseProvider(file: vaultURL, key: key, registerMigrationsHandler: Migrations.v3Migrations)) - } - - func testV3MigrationResultsInNoDataIntegrityIssues() throws { + func testV3MigrationCleansUpOrphanedRecords_andResultsInNoDataIntegrityIssues() throws { // Given let failingMigration: (inout DatabaseMigrator) throws -> Void = { migrator in migrator.registerMigration("v3") { database in @@ -78,34 +64,46 @@ final class DataBrokerProtectionDatabaseProviderTests: XCTestCase { XCTAssertThrowsError(try DefaultDataBrokerProtectionDatabaseProvider(file: vaultURL, key: key, registerMigrationsHandler: failingMigration)) - // When - Then + // When XCTAssertNoThrow(try DefaultDataBrokerProtectionDatabaseProvider(file: vaultURL, key: key, registerMigrationsHandler: Migrations.v3Migrations)) - // When - Then + // Then XCTAssertNoThrow(try DefaultDataBrokerProtectionDatabaseProvider(file: vaultURL, key: key, registerMigrationsHandler: passingMigration)) } - func testV3MigrationRecreatesTablesAsExpectedwWithCascadingDeletes() throws { + func testV3MigrationRecreatesTablesWithCascadingDeletes_andDeletingProfileQueryDeletesDependentRecords() throws { // Given - let failingMigration: (inout DatabaseMigrator) throws -> Void = { migrator in - migrator.registerMigration("v3") { database in - try database.checkForeignKeys() - } - } - - let passingMigration: (inout DatabaseMigrator) throws -> Void = { migrator in - migrator.registerMigration("v4") { database in - try database.checkForeignKeys() - } - } - - XCTAssertThrowsError(try DefaultDataBrokerProtectionDatabaseProvider(file: vaultURL, key: key, registerMigrationsHandler: failingMigration)) - - // When - Then XCTAssertNoThrow(try DefaultDataBrokerProtectionDatabaseProvider(file: vaultURL, key: key, registerMigrationsHandler: Migrations.v3Migrations)) - - // When - Then - // TODO: Delete data here and ensure cascading deletes work as expected + XCTAssertEqual(try sut.fetchAllScans().filter { $0.profileQueryId == 43 }.count, 50) + let allBrokerIds = try sut.fetchAllBrokers().map { $0.id! } + var allExtractedProfiles = try allBrokerIds.flatMap { try sut.fetchExtractedProfiles(for: $0, with:43) } + let extractedProfileId = allExtractedProfiles.first!.id + var optOutAttempt = try sut.fetchAttemptInformation(for: extractedProfileId!) + var allOptOuts = try allBrokerIds.flatMap { try sut.fetchOptOuts(brokerId: $0, profileQueryId:43) } + var allScanHistoryEvents = try allBrokerIds.flatMap { try sut.fetchScanEvents(brokerId: $0, profileQueryId:43) } + var allOptOutHistoryEvents = try allBrokerIds.flatMap { try sut.fetchOptOutEvents(brokerId: $0, profileQueryId:43) } + XCTAssertNotNil(optOutAttempt) + XCTAssertEqual(allExtractedProfiles.count, 1) + XCTAssertEqual(allOptOuts.count, 1) + XCTAssertEqual(allScanHistoryEvents.count, 656) + XCTAssertEqual(allOptOutHistoryEvents.count, 4) + let profileQuery = try sut.fetchProfileQuery(with: 43)! + + // When + try sut.delete(profileQuery) + + // Then + XCTAssertEqual(try sut.fetchAllScans().filter { $0.profileQueryId == 43 }.count, 0) + allExtractedProfiles = try allBrokerIds.flatMap { try sut.fetchExtractedProfiles(for: $0, with:43) } + optOutAttempt = try sut.fetchAttemptInformation(for: extractedProfileId!) + allOptOuts = try allBrokerIds.flatMap { try sut.fetchOptOuts(brokerId: $0, profileQueryId:43) } + allScanHistoryEvents = try allBrokerIds.flatMap { try sut.fetchScanEvents(brokerId: $0, profileQueryId:43) } + allOptOutHistoryEvents = try allBrokerIds.flatMap { try sut.fetchOptOutEvents(brokerId: $0, profileQueryId:43) } + XCTAssertNil(optOutAttempt) + XCTAssertEqual(allExtractedProfiles.count, 0) + XCTAssertEqual(allOptOuts.count, 0) + XCTAssertEqual(allScanHistoryEvents.count, 0) + XCTAssertEqual(allOptOutHistoryEvents.count, 0) } func testDeleteAllDataSucceedsInRemovingAllData() throws { @@ -115,6 +113,15 @@ final class DataBrokerProtectionDatabaseProviderTests: XCTestCase { } } +extension DataBrokerProtectionDatabaseProvider { + func fetchAllProfiles() throws -> [ProfileDB] { + return try db.read { db in + let profiles = try ProfileDB.fetchAll(db, sql: "SELECT * FROM profile") + return profiles + } + } +} + extension DatabaseWriter { func allTablesAreEmpty() throws -> Bool {