Merge branch 'charlesmchen/decryptDeduplicationByProcessing' into release/5.20.4

Author: charlesmchen-signal

NotificationServiceExtension/NSEEnvironment.swift

@@ -157,8 +157,6 @@ class NSEEnvironment: Dependencies {
     }
 
     public static func verifyDBKeysAvailable() -> UNNotificationContent? {
-        AssertIsOnMainThread()
-
         guard !StorageCoordinator.hasGrdbFile || !GRDBDatabaseStorageAdapter.isKeyAccessible else { return nil }
 
         Logger.info("Database password is not accessible, posting generic notification.")

SignalServiceKit/Resources/schema.sql

@@ -1237,17 +1237,10 @@ CREATE
     TABLE
         IF NOT EXISTS "MessageDecryptDeduplication" (
             "id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL
-            ,"serviceTimestamp" INTEGER NOT NULL
             ,"serverGuid" TEXT NOT NULL
         )
 ;
 
-CREATE
-    INDEX "MessageDecryptDeduplication_serviceTimestamp"
-        ON "MessageDecryptDeduplication"("serviceTimestamp"
-        )
-;
-
 CREATE
     INDEX "MessageDecryptDeduplication_serverGuid"
         ON "MessageDecryptDeduplication"("serverGuid"

SignalServiceKit/src/Messages/MessageProcessor.swift

@@ -473,11 +473,9 @@ class MessageDecryptDeduplicationRecord: Codable, FetchableRecord, PersistableRe
     static let databaseTableName = "MessageDecryptDeduplication"
 
     var id: Int64?
-    let serviceTimestamp: UInt64
     let serverGuid: String
 
-    init(serviceTimestamp: UInt64, serverGuid: String) {
-        self.serviceTimestamp = serviceTimestamp
+    init(serverGuid: String) {
         self.serverGuid = serverGuid
     }
 
@@ -496,20 +494,26 @@ class MessageDecryptDeduplicationRecord: Codable, FetchableRecord, PersistableRe
         transaction: SDSAnyWriteTransaction,
         skipCull: Bool = false
     ) -> Outcome {
-        deduplicate(serviceTimestamp: encryptedEnvelope.serverTimestamp,
+        deduplicate(envelopeTimestamp: encryptedEnvelope.timestamp,
+                    serviceTimestamp: encryptedEnvelope.serverTimestamp,
                     serverGuid: encryptedEnvelope.serverGuid,
                     transaction: transaction,
                     skipCull: skipCull)
     }
 
     public static func deduplicate(
+        envelopeTimestamp: UInt64,
         serviceTimestamp: UInt64,
         serverGuid: String?,
         transaction: SDSAnyWriteTransaction,
         skipCull: Bool = false
     ) -> Outcome {
+        guard envelopeTimestamp > 0 else {
+            owsFailDebug("Invalid envelopeTimestamp.")
+            return .nonDuplicate
+        }
         guard serviceTimestamp > 0 else {
-            owsFailDebug("Missing serviceTimestamp.")
+            owsFailDebug("Invalid serviceTimestamp.")
             return .nonDuplicate
         }
         guard let serverGuid = serverGuid?.nilIfEmpty else {
@@ -527,21 +531,20 @@ class MessageDecryptDeduplicationRecord: Codable, FetchableRecord, PersistableRe
                 return try Bool.fetchOne(transaction.unwrapGrdbWrite.database, sql: sql, arguments: arguments) ?? false
             }()
             guard !isDuplicate else {
-                Logger.warn("Discarding duplicate envelope with serviceTimestamp: \(serviceTimestamp), serverGuid: \(serverGuid)")
+                Logger.warn("Discarding duplicate envelope with envelopeTimestamp: \(envelopeTimestamp), serviceTimestamp: \(serviceTimestamp), serverGuid: \(serverGuid)")
                 return .duplicate
             }
 
             // No existing record found. Create a new one and insert it.
-            let record = MessageDecryptDeduplicationRecord(serviceTimestamp: serviceTimestamp,
-                                                           serverGuid: serverGuid)
+            let record = MessageDecryptDeduplicationRecord(serverGuid: serverGuid)
             try record.insert(transaction.unwrapGrdbWrite.database)
 
             if !skipCull, shouldCull() {
-                cull(latestServiceTimestamp: serviceTimestamp, transaction: transaction)
+                cull(transaction: transaction)
             }
 
             if DebugFlags.internalLogging {
-                Logger.info("Proceeding with serviceTimestamp: \(serviceTimestamp), serverGuid: \(serverGuid)")
+                Logger.info("Proceeding with envelopeTimestamp: \(envelopeTimestamp), serviceTimestamp: \(serviceTimestamp), serverGuid: \(serverGuid)")
             }
             return .nonDuplicate
         } catch {
@@ -553,34 +556,23 @@ class MessageDecryptDeduplicationRecord: Codable, FetchableRecord, PersistableRe
     }
 
     static let maxRecordCount: UInt = 1000
-    static let maxRecordAgeMs: UInt64 = 5 * kMinuteInMs
-
-    private static func cull(latestServiceTimestamp: UInt64,
-                             transaction: SDSAnyWriteTransaction) {
-        let count1 = recordCount(transaction: transaction)
-
-        // Client and service time might not match; use service timestamps for
-        // all record bookkeeping.
-        let recordExpirationTimestamp = latestServiceTimestamp - maxRecordAgeMs
-        let sql = """
-            DELETE FROM MessageDecryptDeduplication
-            WHERE serviceTimestamp < ?;
-        """
-        transaction.unwrapGrdbWrite.executeUpdate(sql: sql, arguments: [recordExpirationTimestamp])
-
-        let count2 = recordCount(transaction: transaction)
-        if count1 != count2 {
-            Logger.info("Culled by timestamp: \(count1) -> \(count2)")
-        }
 
-        guard count2 > maxRecordCount else {
+    static let cullCount = AtomicUInt(0)
+
+    private static func cull(transaction: SDSAnyWriteTransaction) {
+
+        cullCount.increment()
+
+        let oldCount = recordCount(transaction: transaction)
+
+        guard oldCount > maxRecordCount else {
             return
         }
 
         do {
             // It is sufficient to cull by record count in batches.
             // The batch size must be larger than our cull frequency to bound total record count.
-            let cullCount: Int = min(Int(cullFrequency) * 2, Int(count2) - Int(maxRecordCount))
+            let cullCount: Int = min(Int(cullFrequency) * 2, Int(oldCount) - Int(maxRecordCount))
 
             Logger.info("Culling \(cullCount) records.")
 
@@ -592,11 +584,11 @@ class MessageDecryptDeduplicationRecord: Codable, FetchableRecord, PersistableRe
                 try record.delete(transaction.unwrapGrdbWrite.database)
             }
 
-            let count3 = recordCount(transaction: transaction)
-            if count2 != count3 {
-                Logger.info("Culled by count: \(count2) -> \(count3)")
+            let newCount = recordCount(transaction: transaction)
+            if oldCount != newCount {
+                Logger.info("Culled by count: \(oldCount) -> \(newCount)")
             }
-            owsAssertDebug(count3 <= maxRecordCount)
+            owsAssertDebug(newCount <= maxRecordCount)
         } catch {
             owsFailDebug("Error: \(error)")
         }

SignalServiceKit/src/Storage/Database/GRDBSchemaMigrator.swift

@@ -113,7 +113,7 @@ public class GRDBSchemaMigrator: NSObject {
         case updateMessageSendLogColumnTypes
         case addRecordTypeIndex
         case tunedConversationLoadIndices
-        case messageDecryptDeduplication3
+        case messageDecryptDeduplicationV5
 
         // NOTE: Every time we add a migration id, consider
         // incrementing grdbSchemaVersionLatest.
@@ -1446,7 +1446,7 @@ public class GRDBSchemaMigrator: NSObject {
             }
         }
 
-        migrator.registerMigration(MigrationId.messageDecryptDeduplication3.rawValue) { db in
+        migrator.registerMigration(MigrationId.messageDecryptDeduplicationV5.rawValue) { db in
             do {
                 if try db.tableExists("MessageDecryptDeduplication") {
                     try db.drop(table: "MessageDecryptDeduplication")
@@ -1455,18 +1455,10 @@ public class GRDBSchemaMigrator: NSObject {
                 try db.create(table: "MessageDecryptDeduplication") { table in
                     table.autoIncrementedPrimaryKey("id")
                         .notNull()
-                    table.column("serviceTimestamp", .integer)
-                        .notNull()
                     table.column("serverGuid", .text)
                         .notNull()
                 }
 
-                try db.create(
-                    index: "MessageDecryptDeduplication_serviceTimestamp",
-                    on: "MessageDecryptDeduplication",
-                    columns: ["serviceTimestamp"]
-                )
-
                 try db.create(
                     index: "MessageDecryptDeduplication_serverGuid",
                     on: "MessageDecryptDeduplication",

SignalServiceKit/src/Util/Atomics.swift

@@ -110,6 +110,50 @@ public class AtomicUInt: NSObject {
 
 // MARK: -
 
+@objc
+public class AtomicUInt64: NSObject {
+    private let value: AtomicValue<UInt64>
+
+    @objc @available(swift, obsoleted: 1.0)
+    public required convenience init(_ value: UInt64) {
+        self.init(value, lock: nil)
+    }
+
+    public required init(_ value: UInt64 = 0, lock: AtomicLock? = nil) {
+        self.value = AtomicValue(value, lock: lock)
+    }
+
+    @objc
+    public func get() -> UInt64 {
+        value.get()
+    }
+
+    @objc
+    public func set(_ value: UInt64) {
+        self.value.set(value)
+    }
+
+    @discardableResult
+    @objc
+    public func increment() -> UInt64 {
+        value.map { $0 + 1 }
+    }
+
+    @discardableResult
+    @objc
+    public func decrementOrZero() -> UInt64 {
+        value.map { max($0, 1) - 1 }
+    }
+
+    @discardableResult
+    @objc
+    public func add(_ delta: UInt64) -> UInt64 {
+        value.map { $0 + delta }
+    }
+}
+
+// MARK: -
+
 public final class AtomicValue<T> {
     private let lock: AtomicLock
     private var value: T
@@ -307,7 +351,7 @@ public class AtomicArray<T> {
     public func pushTail(_ value: T) {
         append(value)
     }
-    
+
     public var count: Int {
         lock.perform { values.count }
     }
@@ -374,7 +418,7 @@ public class AtomicDictionary<Key: Hashable, Value> {
             return result
         }
     }
-    
+
     public var count: Int {
         lock.perform { values.count }
     }