Rerun orphan cleanup as last migration step, add documentation, impro…

…ve tests

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 {

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()
     }

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 {