From 085a1a59aa31e68bd562df56a09bf82dfd4138ca Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 6 Oct 2023 18:03:12 +1100 Subject: [PATCH] Fixed an issue where the messages might not get reprocessed when they should Dropped the auto-incrementing id from the SnodeReceivedMessageInfo Changed the 'key, hash' from a uniqueKey to a primaryKey to allow "upsert" behaviours to work --- Session.xcodeproj/project.pbxproj | 4 ++ SessionMessagingKit/Configuration.swift | 10 +-- .../Sending & Receiving/Pollers/Poller.swift | 2 +- SessionSnodeKit/Configuration.swift | 12 ++-- .../_001_InitialSetupMigration.swift | 2 +- ...ddSnodeReveivedMessageInfoPrimaryKey.swift | 72 +++++++++++++++++++ .../Models/SnodeReceivedMessageInfo.swift | 15 +--- SessionSnodeKit/Types/SnodeAPINamespace.swift | 11 ++- SessionUIKit/Configuration.swift | 6 +- SessionUtilitiesKit/Configuration.swift | 8 ++- 10 files changed, 107 insertions(+), 35 deletions(-) create mode 100644 SessionSnodeKit/Database/Migrations/_005_AddSnodeReveivedMessageInfoPrimaryKey.swift diff --git a/Session.xcodeproj/project.pbxproj b/Session.xcodeproj/project.pbxproj index cce1f839f..4782e4899 100644 --- a/Session.xcodeproj/project.pbxproj +++ b/Session.xcodeproj/project.pbxproj @@ -648,6 +648,7 @@ FD6A7A692818BE7300035AC1 /* RetrieveDefaultOpenGroupRoomsJob.swift in Sources */ = {isa = PBXBuildFile; fileRef = FD6A7A682818BE7300035AC1 /* RetrieveDefaultOpenGroupRoomsJob.swift */; }; FD6A7A6B2818C17C00035AC1 /* UpdateProfilePictureJob.swift in Sources */ = {isa = PBXBuildFile; fileRef = FD6A7A6A2818C17C00035AC1 /* UpdateProfilePictureJob.swift */; }; FD6A7A6D2818C61500035AC1 /* _002_SetupStandardJobs.swift in Sources */ = {isa = PBXBuildFile; fileRef = FD6A7A6C2818C61500035AC1 /* _002_SetupStandardJobs.swift */; }; + FD6DF00B2ACFE40D0084BA4C /* _005_AddSnodeReveivedMessageInfoPrimaryKey.swift in Sources */ = {isa = PBXBuildFile; fileRef = FD6DF00A2ACFE40D0084BA4C /* _005_AddSnodeReveivedMessageInfoPrimaryKey.swift */; }; FD6E4C8A2A1AEE4700C7C243 /* LegacyUnsubscribeRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = FD6E4C892A1AEE4700C7C243 /* LegacyUnsubscribeRequest.swift */; }; FD705A92278D051200F16121 /* ReusableView.swift in Sources */ = {isa = PBXBuildFile; fileRef = FD705A91278D051200F16121 /* ReusableView.swift */; }; FD7115EB28C5D78E00B47552 /* ThreadSettingsViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = FD7115EA28C5D78E00B47552 /* ThreadSettingsViewModel.swift */; }; @@ -1759,6 +1760,7 @@ FD6A7A682818BE7300035AC1 /* RetrieveDefaultOpenGroupRoomsJob.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RetrieveDefaultOpenGroupRoomsJob.swift; sourceTree = ""; }; FD6A7A6A2818C17C00035AC1 /* UpdateProfilePictureJob.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UpdateProfilePictureJob.swift; sourceTree = ""; }; FD6A7A6C2818C61500035AC1 /* _002_SetupStandardJobs.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = _002_SetupStandardJobs.swift; sourceTree = ""; }; + FD6DF00A2ACFE40D0084BA4C /* _005_AddSnodeReveivedMessageInfoPrimaryKey.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = _005_AddSnodeReveivedMessageInfoPrimaryKey.swift; sourceTree = ""; }; FD6E4C892A1AEE4700C7C243 /* LegacyUnsubscribeRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LegacyUnsubscribeRequest.swift; sourceTree = ""; }; FD705A91278D051200F16121 /* ReusableView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReusableView.swift; sourceTree = ""; }; FD7115EA28C5D78E00B47552 /* ThreadSettingsViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ThreadSettingsViewModel.swift; sourceTree = ""; }; @@ -3670,6 +3672,7 @@ FD6A7A6C2818C61500035AC1 /* _002_SetupStandardJobs.swift */, FD17D7A327F40F8100122BE0 /* _003_YDBToGRDBMigration.swift */, FD39353528F7C3390084DADA /* _004_FlagMessageHashAsDeletedOrInvalid.swift */, + FD6DF00A2ACFE40D0084BA4C /* _005_AddSnodeReveivedMessageInfoPrimaryKey.swift */, ); path = Migrations; sourceTree = ""; @@ -5765,6 +5768,7 @@ FD39353628F7C3390084DADA /* _004_FlagMessageHashAsDeletedOrInvalid.swift in Sources */, FDF8489429405C1B007DCAE5 /* SnodeAPI.swift in Sources */, FDF848C829405C5B007DCAE5 /* ONSResolveRequest.swift in Sources */, + FD6DF00B2ACFE40D0084BA4C /* _005_AddSnodeReveivedMessageInfoPrimaryKey.swift in Sources */, C3C2A5C2255385EE00C340D1 /* Configuration.swift in Sources */, FDF848C929405C5B007DCAE5 /* SnodeRequest.swift in Sources */, FDF848CF29405C5B007DCAE5 /* SendMessageRequest.swift in Sources */, diff --git a/SessionMessagingKit/Configuration.swift b/SessionMessagingKit/Configuration.swift index 8fb465756..57b400250 100644 --- a/SessionMessagingKit/Configuration.swift +++ b/SessionMessagingKit/Configuration.swift @@ -10,23 +10,23 @@ public enum SNMessagingKit: MigratableTarget { // Just to make the external API [ _001_InitialSetupMigration.self, _002_SetupStandardJobs.self - ], + ], // Initial DB Creation [ _003_YDBToGRDBMigration.self - ], + ], // YDB to GRDB Migration [ _004_RemoveLegacyYDB.self - ], + ], // Legacy DB removal [ _005_FixDeletedMessageReadState.self, _006_FixHiddenModAdminSupport.self, _007_HomeQueryOptimisationIndexes.self - ], + ], // Add job priorities [ _008_EmojiReacts.self, _009_OpenGroupPermission.self, _010_AddThreadIdToFTS.self - ], // Add job priorities + ], // Fix thread FTS [ _011_AddPendingReadReceipts.self, _012_AddFTSIfNeeded.self, diff --git a/SessionMessagingKit/Sending & Receiving/Pollers/Poller.swift b/SessionMessagingKit/Sending & Receiving/Pollers/Poller.swift index b4c89e410..91b43e9e1 100644 --- a/SessionMessagingKit/Sending & Receiving/Pollers/Poller.swift +++ b/SessionMessagingKit/Sending & Receiving/Pollers/Poller.swift @@ -263,7 +263,7 @@ public class Poller { let lastHashes: [String] = namespacedResults .compactMap { $0.value.data?.lastHash } let otherKnownHashes: [String] = namespacedResults - .filter { $0.key.shouldDedupeMessages } + .filter { $0.key.shouldFetchSinceLastHash } .compactMap { $0.value.data?.messages.map { $0.info.hash } } .reduce([], +) var messageCount: Int = 0 diff --git a/SessionSnodeKit/Configuration.swift b/SessionSnodeKit/Configuration.swift index 4bef7635a..1742647be 100644 --- a/SessionSnodeKit/Configuration.swift +++ b/SessionSnodeKit/Configuration.swift @@ -12,14 +12,18 @@ public enum SNSnodeKit: MigratableTarget { // Just to make the external API nice [ _001_InitialSetupMigration.self, _002_SetupStandardJobs.self - ], + ], // Initial DB Creation [ _003_YDBToGRDBMigration.self - ], + ], // YDB to GRDB Migration [ _004_FlagMessageHashAsDeletedOrInvalid.self - ], - [] // Add job priorities + ], // Legacy DB removal + [], // Add job priorities + [], // Fix thread FTS + [ + _005_AddSnodeReveivedMessageInfoPrimaryKey.self + ] ] ) } diff --git a/SessionSnodeKit/Database/Migrations/_001_InitialSetupMigration.swift b/SessionSnodeKit/Database/Migrations/_001_InitialSetupMigration.swift index 8ddece9a0..e5d449b03 100644 --- a/SessionSnodeKit/Database/Migrations/_001_InitialSetupMigration.swift +++ b/SessionSnodeKit/Database/Migrations/_001_InitialSetupMigration.swift @@ -40,7 +40,7 @@ enum _001_InitialSetupMigration: Migration { } try db.create(table: SnodeReceivedMessageInfo.self) { t in - t.column(.id, .integer) + t.deprecatedColumn(name: "id", .integer) // stringlint:disable .notNull() .primaryKey(autoincrement: true) t.column(.key, .text) diff --git a/SessionSnodeKit/Database/Migrations/_005_AddSnodeReveivedMessageInfoPrimaryKey.swift b/SessionSnodeKit/Database/Migrations/_005_AddSnodeReveivedMessageInfoPrimaryKey.swift new file mode 100644 index 000000000..b6dd7435d --- /dev/null +++ b/SessionSnodeKit/Database/Migrations/_005_AddSnodeReveivedMessageInfoPrimaryKey.swift @@ -0,0 +1,72 @@ +// Copyright © 2023 Rangeproof Pty Ltd. All rights reserved. + +import Foundation +import GRDB +import SessionUtilitiesKit + +enum _005_AddSnodeReveivedMessageInfoPrimaryKey: Migration { + static let target: TargetMigrations.Identifier = .snodeKit + static let identifier: String = "AddSnodeReveivedMessageInfoPrimaryKey" // stringlint:disable + static let needsConfigSync: Bool = false + static let fetchedTables: [(TableRecord & FetchableRecord).Type] = [SnodeReceivedMessageInfo.self] + static let createdOrAlteredTables: [(TableRecord & FetchableRecord).Type] = [SnodeReceivedMessageInfo.self] + + /// This migration adds a flat to the `SnodeReceivedMessageInfo` so that when deleting interactions we can + /// ignore their hashes when subsequently trying to fetch new messages (which results in the storage server returning + /// messages from the beginning of time) + static let minExpectedRunDuration: TimeInterval = 0.2 + + static func migrate(_ db: Database) throws { + // SQLite doesn't support adding a new primary key after creation so we need to create a new table with + // the setup we want, copy data from the old table over, drop the old table and rename the new table + struct TmpSnodeReceivedMessageInfo: Codable, TableRecord, FetchableRecord, PersistableRecord, ColumnExpressible { + static var databaseTableName: String { "tmpSnodeReceivedMessageInfo" } + + typealias Columns = CodingKeys + enum CodingKeys: String, CodingKey, ColumnExpression { + case key + case hash + case expirationDateMs + case wasDeletedOrInvalid + } + + let key: String + let hash: String + let expirationDateMs: Int64 + var wasDeletedOrInvalid: Bool? + } + + try db.create(table: TmpSnodeReceivedMessageInfo.self) { t in + t.column(.key, .text).notNull() + t.column(.hash, .text).notNull() + t.column(.expirationDateMs, .integer).notNull() + t.column(.wasDeletedOrInvalid, .boolean) + + t.primaryKey([.key, .hash]) + } + + // Insert into the new table, drop the old table and rename the new table to be the old one + let tmpInfo: TypedTableAlias = TypedTableAlias() + let info: TypedTableAlias = TypedTableAlias() + try db.execute(literal: """ + INSERT INTO \(tmpInfo) + SELECT \(info[.key]), \(info[.hash]), \(info[.expirationDateMs]), \(info[.wasDeletedOrInvalid]) + FROM \(info) + """) + + try db.drop(table: SnodeReceivedMessageInfo.self) + try db.rename( + table: TmpSnodeReceivedMessageInfo.databaseTableName, + to: SnodeReceivedMessageInfo.databaseTableName + ) + + // Need to create the indexes separately from creating 'TmpGroupMember' to ensure they + // have the correct names + try db.createIndex(on: SnodeReceivedMessageInfo.self, columns: [.key]) + try db.createIndex(on: SnodeReceivedMessageInfo.self, columns: [.hash]) + try db.createIndex(on: SnodeReceivedMessageInfo.self, columns: [.expirationDateMs]) + try db.createIndex(on: SnodeReceivedMessageInfo.self, columns: [.wasDeletedOrInvalid]) + + Storage.update(progress: 1, for: self, in: target) + } +} diff --git a/SessionSnodeKit/Database/Models/SnodeReceivedMessageInfo.swift b/SessionSnodeKit/Database/Models/SnodeReceivedMessageInfo.swift index f867542eb..4c5a7dc1f 100644 --- a/SessionSnodeKit/Database/Models/SnodeReceivedMessageInfo.swift +++ b/SessionSnodeKit/Database/Models/SnodeReceivedMessageInfo.swift @@ -9,17 +9,12 @@ public struct SnodeReceivedMessageInfo: Codable, FetchableRecord, MutablePersist public typealias Columns = CodingKeys public enum CodingKeys: String, CodingKey, ColumnExpression { - case id case key case hash case expirationDateMs case wasDeletedOrInvalid } - /// The `id` value is auto incremented by the database, if the `Job` hasn't been inserted into - /// the database yet this value will be `nil` - public var id: Int64? = nil - /// The key this message hash is associated to /// /// This will be a combination of {address}.{port}.{publicKey} for new rows and just the {publicKey} for legacy rows @@ -41,12 +36,6 @@ public struct SnodeReceivedMessageInfo: Codable, FetchableRecord, MutablePersist /// /// **Note:** When retrieving the `lastNotExpired` we will ignore any entries where this flag is true public var wasDeletedOrInvalid: Bool? - - // MARK: - Custom Database Interaction - - public mutating func didInsert(_ inserted: InsertionSuccess) { - self.id = inserted.rowID - } } // MARK: - Convenience @@ -133,7 +122,7 @@ public extension SnodeReceivedMessageInfo { ) .filter(SnodeReceivedMessageInfo.Columns.key == key(for: snode, publicKey: publicKey, namespace: namespace)) .filter(SnodeReceivedMessageInfo.Columns.expirationDateMs > SnodeAPI.currentOffsetTimestampMs()) - .order(SnodeReceivedMessageInfo.Columns.id.desc) + .order(Column.rowID.desc) .fetchOne(db) // If we have a non-legacy hash then return it immediately (legacy hashes had a different @@ -146,7 +135,7 @@ public extension SnodeReceivedMessageInfo { SnodeReceivedMessageInfo.Columns.wasDeletedOrInvalid == false ) .filter(SnodeReceivedMessageInfo.Columns.key == publicKey) - .order(SnodeReceivedMessageInfo.Columns.id.desc) + .order(Column.rowID.desc) .fetchOne(db) } } diff --git a/SessionSnodeKit/Types/SnodeAPINamespace.swift b/SessionSnodeKit/Types/SnodeAPINamespace.swift index 73dd9182d..f22a4fcae 100644 --- a/SessionSnodeKit/Types/SnodeAPINamespace.swift +++ b/SessionSnodeKit/Types/SnodeAPINamespace.swift @@ -1,4 +1,6 @@ // Copyright © 2022 Rangeproof Pty Ltd. All rights reserved. +// +// stringlint:disable import Foundation @@ -40,12 +42,9 @@ public extension SnodeAPI { public var shouldFetchSinceLastHash: Bool { true } /// This flag indicates whether we should dedupe messages from the specified namespace, when `true` we will - /// store a `SnodeReceivedMessageInfo` record for the message and check for a matching record whenever - /// we receive a message from this namespace - /// - /// **Note:** An additional side-effect of this flag is that when we poll for messages from the specified namespace - /// we will always retrieve **all** messages from the namespace (instead of just new messages since the last one - /// we have seen) + /// attempt to `insert` a `SnodeReceivedMessageInfo` record (which will fail if we had already processed this + /// message previously), when `false` we will still `upsert` a record so we don't run into the unique constraint allowing + /// re-processing of a previously processed message public var shouldDedupeMessages: Bool { switch self { case .`default`, .legacyClosedGroup: return true diff --git a/SessionUIKit/Configuration.swift b/SessionUIKit/Configuration.swift index 92b5c072d..49fe2a00c 100644 --- a/SessionUIKit/Configuration.swift +++ b/SessionUIKit/Configuration.swift @@ -13,10 +13,12 @@ public enum SNUIKit: MigratableTarget { // SNUIKit migrations [], // Initial DB Creation [], // YDB to GRDB Migration - [], // YDB Removal + [], // Legacy DB removal [ _001_ThemePreferences.self - ] // Add job priorities + ], // Add job priorities + [], // Fix thread FTS + [] ] ) } diff --git a/SessionUtilitiesKit/Configuration.swift b/SessionUtilitiesKit/Configuration.swift index 3cad4b5e8..b78a5a8e7 100644 --- a/SessionUtilitiesKit/Configuration.swift +++ b/SessionUtilitiesKit/Configuration.swift @@ -19,12 +19,14 @@ public enum SNUtilitiesKit: MigratableTarget { // Just to make the external API _001_InitialSetupMigration.self, _002_SetupStandardJobs.self, _003_YDBToGRDBMigration.self - ], - [], // Other DB migrations + ], // Initial DB Creation + [], // YDB to GRDB Migration [], // Legacy DB removal [ _004_AddJobPriority.self - ] + ], // Add job priorities + [], // Fix thread FTS + [] ] ) }