From efaa46abcbb01bec5ff5ad264bfed048a034cc91 Mon Sep 17 00:00:00 2001 From: Harry <109690906+harry-signal@users.noreply.github.com> Date: Fri, 4 Oct 2024 16:11:58 -0700 Subject: [PATCH] Access database connection directly in AttachmentStore --- .../AttachmentStore/AttachmentStoreImpl.swift | 441 +++--------------- .../AttachmentStoreTests.swift | 56 --- .../AttachmentDownloadQueueDBTests.swift | 1 - .../AttachmentDownloadStoreTests.swift | 1 - .../OrphanedAttachmentCleanerTest.swift | 1 - 5 files changed, 61 insertions(+), 439 deletions(-) diff --git a/SignalServiceKit/Messages/Attachments/V2/AttachmentStore/AttachmentStoreImpl.swift b/SignalServiceKit/Messages/Attachments/V2/AttachmentStore/AttachmentStoreImpl.swift index 5db04b4d00..94b54bbea8 100644 --- a/SignalServiceKit/Messages/Attachments/V2/AttachmentStore/AttachmentStoreImpl.swift +++ b/SignalServiceKit/Messages/Attachments/V2/AttachmentStore/AttachmentStoreImpl.swift @@ -4,238 +4,26 @@ // import Foundation -public import GRDB +import GRDB public class AttachmentStoreImpl: AttachmentStore { public init() {} - public func fetchReferences( - owners: [AttachmentReference.OwnerId], - tx: DBReadTransaction - ) -> [AttachmentReference] { - fetchReferences( - owners: owners, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbRead.database, - tx: tx - ) - } - - public func fetch(ids: [Attachment.IDType], tx: DBReadTransaction) -> [Attachment] { - fetch(ids: ids, db: SDSDB.shimOnlyBridge(tx).unwrapGrdbRead.database, tx: tx) - } - - public func fetchAttachment(sha256ContentHash: Data, tx: DBReadTransaction) -> Attachment? { - try? fetchAttachment( - sha256ContentHash: sha256ContentHash, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbRead.database, - tx: tx - ) - } - - public func fetchAttachment( - mediaName: String, - tx: DBReadTransaction - ) -> Attachment? { - try? fetchAttachment( - mediaName: mediaName, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbRead.database, - tx: tx - ) - } - - public func enumerateAllReferences( - toAttachmentId attachmentId: Attachment.IDType, - tx: DBReadTransaction, - block: (AttachmentReference) -> Void - ) throws { - try enumerateAllReferences( - toAttachmentId: attachmentId, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbRead.database, - tx: tx, - block: block - ) - } - - public func enumerateAllAttachments( - tx: DBReadTransaction, - block: (Attachment) throws -> Void - ) throws { - try enumerateAllAttachments( - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbRead.database, - tx: tx, - block: block - ) - } - - public func allQuotedReplyAttachments( - forOriginalAttachmentId originalAttachmentId: Attachment.IDType, - tx: DBReadTransaction - ) throws -> [Attachment] { - return try allQuotedReplyAttachments( - forOriginalAttachmentId: originalAttachmentId, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbRead.database, - tx: tx - ) - } - - // MARK: - Writes - - public func duplicateExistingMessageOwner( - _ existingOwnerSource: AttachmentReference.Owner.MessageSource, - with reference: AttachmentReference, - newOwnerMessageRowId: Int64, - newOwnerThreadRowId: Int64, - tx: DBWriteTransaction - ) throws { - try duplicateExistingMessageOwner( - existingOwnerSource, - with: reference, - newOwnerMessageRowId: newOwnerMessageRowId, - newOwnerThreadRowId: newOwnerThreadRowId, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbWrite.database, - tx: tx - ) - } - - public func duplicateExistingThreadOwner( - _ existingOwnerSource: AttachmentReference.Owner.ThreadSource, - with reference: AttachmentReference, - newOwnerThreadRowId: Int64, - tx: DBWriteTransaction - ) throws { - try duplicateExistingThreadOwner( - existingOwnerSource, - with: reference, - newOwnerThreadRowId: newOwnerThreadRowId, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbWrite.database, - tx: tx - ) - } - - public func update( - _ reference: AttachmentReference, - withReceivedAtTimestamp receivedAtTimestamp: UInt64, - tx: DBWriteTransaction - ) throws { - try update( - reference, - withReceivedAtTimestamp: receivedAtTimestamp, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbWrite.database, - tx: tx - ) - } - - public func updateAttachmentAsDownloaded( - from source: QueuedAttachmentDownloadRecord.SourceType, - id: Attachment.IDType, - validatedMimeType: String, - streamInfo: Attachment.StreamInfo, - tx: DBWriteTransaction - ) throws { - try self.updateAttachmentAsDownloaded( - from: source, - id: id, - validatedMimeType: validatedMimeType, - streamInfo: streamInfo, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbWrite.database, - tx: tx - ) - } - - public func updateAttachmentAsFailedToDownload( - from source: QueuedAttachmentDownloadRecord.SourceType, - id: Attachment.IDType, - timestamp: UInt64, - tx: DBWriteTransaction - ) throws { - try self.updateAttachmentAsFailedToDownload( - from: source, - id: id, - timestamp: timestamp, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbWrite.database, - tx: tx - ) - } - - public func updateAttachment( - _ attachment: Attachment, - revalidatedContentType contentType: Attachment.ContentType, - mimeType: String, - blurHash: String?, - tx: DBWriteTransaction - ) throws { - try updateAttachment( - attachment, - revalidatedContentType: contentType, - mimeType: mimeType, - blurHash: blurHash, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbWrite.database, - tx: tx - ) - } - - public func addOwner( - _ reference: AttachmentReference.ConstructionParams, - for attachmentId: Attachment.IDType, - tx: DBWriteTransaction - ) throws { - try addOwner( - reference, - for: attachmentId, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbWrite.database, - tx: tx - ) - } - - public func removeOwner( - _ owner: AttachmentReference.OwnerId, - for attachmentId: Attachment.IDType, - tx: DBWriteTransaction - ) throws { - try removeOwner( - owner, - for: attachmentId, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbWrite.database, - tx: tx - ) - } - - public func insert( - _ attachment: Attachment.ConstructionParams, - reference: AttachmentReference.ConstructionParams, - tx: DBWriteTransaction - ) throws { - try insert( - attachment, - reference: reference, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbWrite.database, - tx: tx - ) - } - - public func removeAllThreadOwners(tx: DBWriteTransaction) throws { - try removeAllThreadOwners(db: SDSDB.shimOnlyBridge(tx).unwrapGrdbWrite.database, tx: tx) - } - - // MARK: - Implementation - typealias MessageAttachmentReferenceRecord = AttachmentReference.MessageAttachmentReferenceRecord typealias MessageOwnerTypeRaw = AttachmentReference.MessageOwnerTypeRaw typealias StoryMessageAttachmentReferenceRecord = AttachmentReference.StoryMessageAttachmentReferenceRecord typealias StoryMessageOwnerTypeRaw = AttachmentReference.StoryMessageOwnerTypeRaw typealias ThreadAttachmentReferenceRecord = AttachmentReference.ThreadAttachmentReferenceRecord - func fetchReferences( + public func fetchReferences( owners: [AttachmentReference.OwnerId], - db: GRDB.Database, tx: DBReadTransaction ) -> [AttachmentReference] { return AttachmentReference.recordTypes.flatMap { recordType in return fetchReferences( owners: owners, recordType: recordType, - db: db, tx: tx ) } @@ -244,7 +32,6 @@ public class AttachmentStoreImpl: AttachmentStore { private func fetchReferences( owners: [AttachmentReference.OwnerId], recordType: RecordType.Type, - db: GRDB.Database, tx: DBReadTransaction ) -> [AttachmentReference] { var filterClauses = [String]() @@ -271,7 +58,7 @@ public class AttachmentStoreImpl: AttachmentStore { let sql = "SELECT * FROM \(recordType.databaseTableName) WHERE \(filterClauses.joined(separator: " OR "));" do { var results = try RecordType - .fetchAll(db, sql: sql, arguments: arguments) + .fetchAll(databaseConnection(tx), sql: sql, arguments: arguments) // If we have one owner and are capable of sorting, sort in ascending order. if owners.count == 1, let orderInOwnerKey = RecordType.orderInOwnerKey { @@ -294,15 +81,14 @@ public class AttachmentStoreImpl: AttachmentStore { } } - func fetch( + public func fetch( ids: [Attachment.IDType], - db: GRDB.Database, tx: DBReadTransaction ) -> [Attachment] { do { return try Attachment.Record .fetchAll( - db, + databaseConnection(tx), keys: ids ) .compactMap { record in @@ -317,42 +103,52 @@ public class AttachmentStoreImpl: AttachmentStore { } } - func fetchAttachment( + public func fetchAttachment( + sha256ContentHash: Data, + tx: DBReadTransaction + ) -> Attachment? { + return try? fetchAttachmentThrows(sha256ContentHash: sha256ContentHash, tx: tx) + } + + private func fetchAttachmentThrows( sha256ContentHash: Data, - db: GRDB.Database, tx: DBReadTransaction ) throws -> Attachment? { return try Attachment.Record .filter(Column(Attachment.Record.CodingKeys.sha256ContentHash) == sha256ContentHash) - .fetchOne(db) + .fetchOne(databaseConnection(tx)) .map(Attachment.init(record:)) } - func fetchAttachment( + public func fetchAttachment( + mediaName: String, + tx: DBReadTransaction + ) -> Attachment? { + return try? fetchAttachmentThrows(mediaName: mediaName, tx: tx) + } + + private func fetchAttachmentThrows( mediaName: String, - db: GRDB.Database, tx: DBReadTransaction ) throws -> Attachment? { return try Attachment.Record .filter(Column(Attachment.Record.CodingKeys.mediaName) == mediaName) - .fetchOne(db) + .fetchOne(databaseConnection(tx)) .map(Attachment.init(record:)) } - func allQuotedReplyAttachments( + public func allQuotedReplyAttachments( forOriginalAttachmentId originalAttachmentId: Attachment.IDType, - db: GRDB.Database, tx: DBReadTransaction ) throws -> [Attachment] { return try Attachment.Record .filter(Column(Attachment.Record.CodingKeys.originalAttachmentIdForQuotedReply) == originalAttachmentId) - .fetchAll(db) + .fetchAll(databaseConnection(tx)) .map(Attachment.init(record:)) } - func enumerateAllReferences( + public func enumerateAllReferences( toAttachmentId attachmentId: Attachment.IDType, - db: GRDB.Database, tx: DBReadTransaction, block: (AttachmentReference) -> Void ) throws { @@ -360,19 +156,17 @@ public class AttachmentStoreImpl: AttachmentStore { try enumerateReferences( attachmentId: attachmentId, recordType: recordType, - db: db, tx: tx, block: block ) } } - func enumerateAllAttachments( - db: GRDB.Database, + public func enumerateAllAttachments( tx: DBReadTransaction, block: (Attachment) throws -> Void ) throws { - try Attachment.Record.fetchCursor(db) + try Attachment.Record.fetchCursor(databaseConnection(tx)) .forEach { let attachment = try Attachment(record: $0) try block(attachment) @@ -382,13 +176,12 @@ public class AttachmentStoreImpl: AttachmentStore { private func enumerateReferences( attachmentId: Attachment.IDType, recordType: RecordType.Type, - db: GRDB.Database, tx: DBReadTransaction, block: (AttachmentReference) -> Void ) throws { let cursor = try recordType .filter(recordType.attachmentRowIdColumn == attachmentId) - .fetchCursor(db) + .fetchCursor(databaseConnection(tx)) while let record = try cursor.next() { let reference = try record.asReference() @@ -398,12 +191,11 @@ public class AttachmentStoreImpl: AttachmentStore { // MARK: Writes - func duplicateExistingMessageOwner( + public func duplicateExistingMessageOwner( _ existingOwnerSource: AttachmentReference.Owner.MessageSource, with existingReference: AttachmentReference, newOwnerMessageRowId: Int64, newOwnerThreadRowId: Int64, - db: GRDB.Database, tx: DBWriteTransaction ) throws { var newRecord = MessageAttachmentReferenceRecord( @@ -418,14 +210,13 @@ public class AttachmentStoreImpl: AttachmentStore { throw OWSAssertionError("Copying reference to a message on another thread!") } newRecord.ownerRowId = newOwnerMessageRowId - try newRecord.insert(db) + try newRecord.insert(databaseConnection(tx)) } - func duplicateExistingThreadOwner( + public func duplicateExistingThreadOwner( _ existingOwnerSource: AttachmentReference.Owner.ThreadSource, with existingReference: AttachmentReference, newOwnerThreadRowId: Int64, - db: GRDB.Database, tx: DBWriteTransaction ) throws { var newRecord = ThreadAttachmentReferenceRecord( @@ -433,13 +224,12 @@ public class AttachmentStoreImpl: AttachmentStore { threadSource: existingOwnerSource ) newRecord.ownerRowId = newOwnerThreadRowId - try newRecord.insert(db) + try newRecord.insert(databaseConnection(tx)) } - func update( + public func update( _ reference: AttachmentReference, withReceivedAtTimestamp receivedAtTimestamp: UInt64, - db: GRDB.Database, tx: DBWriteTransaction ) throws { guard SDS.fitsInInt64(receivedAtTimestamp) else { @@ -453,7 +243,7 @@ public class AttachmentStoreImpl: AttachmentStore { let receivedAtTimestampColumn = Column(MessageAttachmentReferenceRecord.CodingKeys.receivedAtTimestamp) let ownerTypeColumn = Column(MessageAttachmentReferenceRecord.CodingKeys.ownerType) let ownerRowIdColumn = Column(MessageAttachmentReferenceRecord.CodingKeys.ownerRowId) - try db.execute( + try databaseConnection(tx).execute( sql: "UPDATE \(MessageAttachmentReferenceRecord.databaseTableName) " + "SET \(receivedAtTimestampColumn.name) = ? " @@ -471,15 +261,14 @@ public class AttachmentStoreImpl: AttachmentStore { } } - private func updateAttachmentAsDownloaded( + public func updateAttachmentAsDownloaded( from source: QueuedAttachmentDownloadRecord.SourceType, id: Attachment.IDType, validatedMimeType: String, streamInfo: Attachment.StreamInfo, - db: GRDB.Database, tx: DBWriteTransaction ) throws { - let existingAttachment = fetch(ids: [id], db: db, tx: tx).first + let existingAttachment = fetch(ids: [id], tx: tx).first guard let existingAttachment else { throw OWSAssertionError("Attachment does not exist") } @@ -488,9 +277,8 @@ public class AttachmentStoreImpl: AttachmentStore { } // Find if there is already an attachment with the same plaintext hash. - let existingRecord = try fetchAttachment( + let existingRecord = try fetchAttachmentThrows( sha256ContentHash: streamInfo.sha256ContentHash, - db: db, tx: tx ).map(Attachment.Record.init(attachment:)) @@ -530,17 +318,16 @@ public class AttachmentStoreImpl: AttachmentStore { } newRecord.sqliteId = id try newRecord.checkAllUInt64FieldsFitInInt64() - try newRecord.update(db) + try newRecord.update(databaseConnection(tx)) } - private func updateAttachmentAsFailedToDownload( + public func updateAttachmentAsFailedToDownload( from source: QueuedAttachmentDownloadRecord.SourceType, id: Attachment.IDType, timestamp: UInt64, - db: GRDB.Database, tx: DBWriteTransaction ) throws { - let existingAttachment = fetch(ids: [id], db: db, tx: tx).first + let existingAttachment = fetch(ids: [id], tx: tx).first guard let existingAttachment else { throw OWSAssertionError("Attachment does not exist") } @@ -574,15 +361,14 @@ public class AttachmentStoreImpl: AttachmentStore { } newRecord.sqliteId = id try newRecord.checkAllUInt64FieldsFitInInt64() - try newRecord.update(db) + try newRecord.update(databaseConnection(tx)) } - private func updateAttachment( + public func updateAttachment( _ attachment: Attachment, revalidatedContentType contentType: Attachment.ContentType, mimeType: String, blurHash: String?, - db: GRDB.Database, tx: DBWriteTransaction ) throws { var newRecord = Attachment.Record( @@ -597,13 +383,12 @@ public class AttachmentStoreImpl: AttachmentStore { try newRecord.checkAllUInt64FieldsFitInInt64() // NOTE: a sqlite trigger handles updating all attachment reference rows // with the new content type. - try newRecord.update(db) + try newRecord.update(databaseConnection(tx)) } - func addOwner( + public func addOwner( _ referenceParams: AttachmentReference.ConstructionParams, for attachmentRowId: Attachment.IDType, - db: GRDB.Database, tx: DBWriteTransaction ) throws { switch referenceParams.owner { @@ -612,20 +397,18 @@ public class AttachmentStoreImpl: AttachmentStore { try insertGlobalThreadAttachmentReference( referenceParams: referenceParams, attachmentRowId: attachmentRowId, - db: db, tx: tx ) default: let referenceRecord = try referenceParams.buildRecord(attachmentRowId: attachmentRowId) try referenceRecord.checkAllUInt64FieldsFitInInt64() - try referenceRecord.insert(db) + try referenceRecord.insert(databaseConnection(tx)) } } - func removeOwner( + public func removeOwner( _ owner: AttachmentReference.OwnerId, for attachmentId: Attachment.IDType, - db: GRDB.Database, tx: DBWriteTransaction ) throws { try AttachmentReference.recordTypes.forEach { recordType in @@ -633,7 +416,6 @@ public class AttachmentStoreImpl: AttachmentStore { owner, for: attachmentId, recordType: recordType, - db: db, tx: tx ) } @@ -643,7 +425,6 @@ public class AttachmentStoreImpl: AttachmentStore { _ owner: AttachmentReference.OwnerId, for attachmentId: Attachment.IDType, recordType: RecordType.Type, - db: GRDB.Database, tx: DBWriteTransaction ) throws { // GRDB's swift query API can't help us here because the AttachmentReference tables @@ -667,23 +448,21 @@ public class AttachmentStoreImpl: AttachmentStore { _ = arguments.append(contentsOf: [ownerType, ownerRowId]) } sql += ";" - try db.execute( + try databaseConnection(tx).execute( sql: sql, arguments: arguments ) } - func insert( + public func insert( _ attachmentParams: Attachment.ConstructionParams, reference referenceParams: AttachmentReference.ConstructionParams, - db: GRDB.Database, tx: DBWriteTransaction ) throws { // Find if there is already an attachment with the same plaintext hash. let existingRecord = try attachmentParams.streamInfo.map { streamInfo in - return try fetchAttachment( + return try fetchAttachmentThrows( sha256ContentHash: streamInfo.sha256ContentHash, - db: db, tx: tx ).map(Attachment.Record.init(attachment:)) } ?? nil @@ -697,7 +476,7 @@ public class AttachmentStoreImpl: AttachmentStore { // Note that this will fail if we have collisions in medianame (unique constraint) // but thats a hash so we just ignore that possibility. - try attachmentRecord.insert(db) + try attachmentRecord.insert(databaseConnection(tx)) guard let attachmentRowId = attachmentRecord.sqliteId else { throw OWSAssertionError("No sqlite id assigned to inserted attachment") @@ -706,7 +485,6 @@ public class AttachmentStoreImpl: AttachmentStore { try addOwner( referenceParams, for: attachmentRowId, - db: db, tx: tx ) } @@ -719,10 +497,9 @@ public class AttachmentStoreImpl: AttachmentStore { private func insertGlobalThreadAttachmentReference( referenceParams: AttachmentReference.ConstructionParams, attachmentRowId: Attachment.IDType, - db: GRDB.Database, tx: DBWriteTransaction ) throws { - + let db = databaseConnection(tx) let ownerRowIdColumn = Column(ThreadAttachmentReferenceRecord.CodingKeys.ownerRowId) let timestampColumn = Column(ThreadAttachmentReferenceRecord.CodingKeys.creationTimestamp) let attachmentRowIdColumn = Column(ThreadAttachmentReferenceRecord.CodingKeys.attachmentRowId) @@ -756,8 +533,8 @@ public class AttachmentStoreImpl: AttachmentStore { } } - func removeAllThreadOwners(db: GRDB.Database, tx: DBWriteTransaction) throws { - try ThreadAttachmentReferenceRecord.deleteAll(db) + public func removeAllThreadOwners(tx: DBWriteTransaction) throws { + try ThreadAttachmentReferenceRecord.deleteAll(databaseConnection(tx)) } } @@ -767,20 +544,6 @@ extension AttachmentStoreImpl: AttachmentUploadStore { attachmentStream: AttachmentStream, info transitTierInfo: Attachment.TransitTierInfo, tx: DBWriteTransaction - ) throws { - try markUploadedToTransitTier( - attachmentStream: attachmentStream, - info: transitTierInfo, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbWrite.database, - tx: tx - ) - } - - func markUploadedToTransitTier( - attachmentStream: AttachmentStream, - info transitTierInfo: Attachment.TransitTierInfo, - db: GRDB.Database, - tx: DBWriteTransaction ) throws { var record = Attachment.Record(attachment: attachmentStream.attachment) record.transitCdnKey = transitTierInfo.cdnKey @@ -790,27 +553,13 @@ extension AttachmentStoreImpl: AttachmentUploadStore { record.transitUnencryptedByteCount = transitTierInfo.unencryptedByteCount record.transitDigestSHA256Ciphertext = transitTierInfo.digestSHA256Ciphertext record.lastTransitDownloadAttemptTimestamp = transitTierInfo.lastDownloadAttemptTimestamp - try record.update(db) + try record.update(databaseConnection(tx)) } public func markTransitTierUploadExpired( attachment: Attachment, info: Attachment.TransitTierInfo, tx: DBWriteTransaction - ) throws { - try markTransitTierUploadExpired( - attachment: attachment, - info: info, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbWrite.database, - tx: tx - ) - } - - func markTransitTierUploadExpired( - attachment: Attachment, - info: Attachment.TransitTierInfo, - db: GRDB.Database, - tx: DBWriteTransaction ) throws { // Refetch the attachment in case the passed in transit tier // info is obsolete. @@ -829,75 +578,35 @@ extension AttachmentStoreImpl: AttachmentUploadStore { record.transitUnencryptedByteCount = nil record.transitDigestSHA256Ciphertext = nil record.lastTransitDownloadAttemptTimestamp = nil - try record.update(db) + try record.update(databaseConnection(tx)) } public func markUploadedToMediaTier( attachmentStream: AttachmentStream, mediaTierInfo: Attachment.MediaTierInfo, tx: DBWriteTransaction - ) throws { - try markUploadedToMediaTier( - attachmentStream: attachmentStream, - mediaTierInfo: mediaTierInfo, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbWrite.database, - tx: tx - ) - } - - func markUploadedToMediaTier( - attachmentStream: AttachmentStream, - mediaTierInfo: Attachment.MediaTierInfo, - db: GRDB.Database, - tx: DBWriteTransaction ) throws { var record = Attachment.Record(attachment: attachmentStream.attachment) record.mediaTierCdnNumber = mediaTierInfo.cdnNumber record.mediaTierUploadEra = mediaTierInfo.uploadEra record.mediaTierUnencryptedByteCount = mediaTierInfo.unencryptedByteCount record.lastMediaTierDownloadAttemptTimestamp = mediaTierInfo.lastDownloadAttemptTimestamp - try record.update(db) + try record.update(databaseConnection(tx)) } public func markThumbnailUploadedToMediaTier( attachmentStream: AttachmentStream, thumbnailMediaTierInfo: Attachment.ThumbnailMediaTierInfo, tx: DBWriteTransaction - ) throws { - try markThumbnailUploadedToMediaTier( - attachmentStream: attachmentStream, - thumbnailMediaTierInfo: thumbnailMediaTierInfo, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbWrite.database, - tx: tx - ) - } - - private func markThumbnailUploadedToMediaTier( - attachmentStream: AttachmentStream, - thumbnailMediaTierInfo: Attachment.ThumbnailMediaTierInfo, - db: GRDB.Database, - tx: DBWriteTransaction ) throws { var record = Attachment.Record(attachment: attachmentStream.attachment) record.thumbnailCdnNumber = thumbnailMediaTierInfo.cdnNumber record.thumbnailUploadEra = thumbnailMediaTierInfo.uploadEra record.lastThumbnailDownloadAttemptTimestamp = thumbnailMediaTierInfo.lastDownloadAttemptTimestamp - try record.update(db) + try record.update(databaseConnection(tx)) } public func upsert(record: AttachmentUploadRecord, tx: DBWriteTransaction) throws { - try upsert( - record: record, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbWrite.database, - tx: tx - ) - } - - public func upsert( - record: AttachmentUploadRecord, - db: GRDB.Database, - tx: any DBWriteTransaction - ) throws { var newRecord = AttachmentUploadRecord(sourceType: record.sourceType, attachmentId: record.attachmentId) newRecord.sqliteId = record.sqliteId newRecord.uploadForm = record.uploadForm @@ -905,56 +614,28 @@ extension AttachmentStoreImpl: AttachmentUploadStore { newRecord.localMetadata = record.localMetadata newRecord.uploadSessionUrl = record.uploadSessionUrl newRecord.attempt = record.attempt - try newRecord.save(db) + try newRecord.save(databaseConnection(tx)) } public func removeRecord( for attachmentId: Attachment.IDType, sourceType: AttachmentUploadRecord.SourceType, - tx: DBWriteTransaction - ) throws { - return try removeRecord( - for: attachmentId, - sourceType: sourceType, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbWrite.database, - tx: tx - ) - } - - public func removeRecord( - for attachmentId: Attachment.IDType, - sourceType: AttachmentUploadRecord.SourceType, - db: GRDB.Database, tx: any DBWriteTransaction ) throws { try AttachmentUploadRecord .filter(Column(AttachmentUploadRecord.CodingKeys.attachmentId) == attachmentId) .filter(Column(AttachmentUploadRecord.CodingKeys.sourceType) == sourceType.rawValue) - .deleteAll(db) + .deleteAll(databaseConnection(tx)) } public func fetchAttachmentUploadRecord( for attachmentId: Attachment.IDType, sourceType: AttachmentUploadRecord.SourceType, tx: DBReadTransaction - ) throws -> AttachmentUploadRecord? { - return try fetchAttachmentUploadRecord( - for: attachmentId, - sourceType: sourceType, - db: SDSDB.shimOnlyBridge(tx).unwrapGrdbRead.database, - tx: tx - ) - } - - public func fetchAttachmentUploadRecord( - for attachmentId: Attachment.IDType, - sourceType: AttachmentUploadRecord.SourceType, - db: GRDB.Database, - tx: any DBReadTransaction ) throws -> AttachmentUploadRecord? { return try AttachmentUploadRecord .filter(Column(AttachmentUploadRecord.CodingKeys.attachmentId) == attachmentId) .filter(Column(AttachmentUploadRecord.CodingKeys.sourceType) == sourceType.rawValue) - .fetchOne(db) + .fetchOne(databaseConnection(tx)) } } diff --git a/SignalServiceKit/Messages/Attachments/V2/AttachmentStore/AttachmentStoreTests.swift b/SignalServiceKit/Messages/Attachments/V2/AttachmentStore/AttachmentStoreTests.swift index 50a398aca0..9767a31629 100644 --- a/SignalServiceKit/Messages/Attachments/V2/AttachmentStore/AttachmentStoreTests.swift +++ b/SignalServiceKit/Messages/Attachments/V2/AttachmentStore/AttachmentStoreTests.swift @@ -30,7 +30,6 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.insert( attachmentParams, reference: referenceParams, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) } @@ -38,12 +37,10 @@ class AttachmentStoreTests: XCTestCase { let (references, attachments) = db.read { tx in let references = attachmentStore.fetchReferences( owners: [.globalThreadWallpaperImage], - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) let attachments = attachmentStore.fetch( ids: references.map(\.attachmentRowId), - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) return (references, attachments) @@ -90,7 +87,6 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.insert( attachmentParams, reference: attachmentReferenceParams, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) attachmentIdToAttachmentParams[id] = attachmentParams @@ -107,12 +103,10 @@ class AttachmentStoreTests: XCTestCase { let (references, attachments) = db.read { tx in let references = attachmentStore.fetchReferences( owners: [.messageBodyAttachment(messageRowId: messageId)], - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) let attachments = attachmentStore.fetch( ids: references.map(\.attachmentRowId), - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) return (references, attachments) @@ -165,18 +159,15 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.insert( attachmentParams1, reference: attachmentReferenceParams1, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) let message1References = attachmentStore.fetchReferences( owners: [.messageBodyAttachment(messageRowId: messageId1)], - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) let message1Attachment = attachmentStore.fetch( ids: message1References.map(\.attachmentRowId), - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ).first! @@ -188,7 +179,6 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.insert( attachmentParams2, reference: attachmentReferenceParams2, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) XCTFail("Should have thrown error!") @@ -208,7 +198,6 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.addOwner( attachmentReferenceParams2, for: message1Attachment.id, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) } @@ -221,22 +210,18 @@ class AttachmentStoreTests: XCTestCase { ) = db.read { tx in let message1References = attachmentStore.fetchReferences( owners: [.messageBodyAttachment(messageRowId: messageId1)], - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) let message1Attachments = attachmentStore.fetch( ids: message1References.map(\.attachmentRowId), - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) let message2References = attachmentStore.fetchReferences( owners: [.messageBodyAttachment(messageRowId: messageId2)], - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) let message2Attachments = attachmentStore.fetch( ids: message1References.map(\.attachmentRowId), - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) return (message1References, message1Attachments, message2References, message2Attachments) @@ -271,14 +256,12 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.insert( attachmentParams1, reference: referenceParams1, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) // Insert which should overwrite the existing row. try attachmentStore.insert( attachmentParams2, reference: referenceParams2, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) } @@ -286,12 +269,10 @@ class AttachmentStoreTests: XCTestCase { let (references, attachments) = db.read { tx in let references = attachmentStore.fetchReferences( owners: [.globalThreadWallpaperImage], - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) let attachments = attachmentStore.fetch( ids: references.map(\.attachmentRowId), - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) return (references, attachments) @@ -329,7 +310,6 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.insert( attachmentParams, reference: attachmentReferenceParams, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) } @@ -348,7 +328,6 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.insert( attachmentParams, reference: attachmentReferenceParams, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) } @@ -384,14 +363,12 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.addOwner( attachmentReferenceParams, for: attachmentRowId, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) } else { try attachmentStore.insert( attachmentParams, reference: attachmentReferenceParams, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) attachmentRowId = InMemoryDB.shimOnlyBridge(tx).db.lastInsertedRowID @@ -405,12 +382,10 @@ class AttachmentStoreTests: XCTestCase { let attachmentId = db.read { tx in let references = attachmentStore.fetchReferences( owners: [.messageBodyAttachment(messageRowId: threadIdAndMessageIds[0].interactionRowId)], - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) let attachment = attachmentStore.fetch( ids: references.map(\.attachmentRowId), - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) return attachment.first!.id @@ -428,7 +403,6 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.insert( Attachment.ConstructionParams.mockPointer(), reference: attachmentReferenceParams, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) } @@ -440,7 +414,6 @@ class AttachmentStoreTests: XCTestCase { try db.read { tx in try attachmentStore.enumerateAllReferences( toAttachmentId: attachmentId, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx, block: { reference in enumeratedCount += 1 @@ -474,7 +447,6 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.insert( .mockStream(), reference: .mock(owner: .thread(.threadWallpaperImage(.init(threadRowId: threadId, creationTimestamp: 0)))), - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) } @@ -484,7 +456,6 @@ class AttachmentStoreTests: XCTestCase { var enumeratedCount = 0 try db.read { tx in try attachmentStore.enumerateAllAttachments( - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx, block: { _ in enumeratedCount += 1 @@ -512,7 +483,6 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.insert( attachmentParams, reference: attachmentReferenceParams, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) } @@ -520,7 +490,6 @@ class AttachmentStoreTests: XCTestCase { var reference = db.read { tx in return attachmentStore.fetchReferences( owners: [.messageBodyAttachment(messageRowId: messageId)], - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ).first! } @@ -539,13 +508,11 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.update( reference, withReceivedAtTimestamp: changedReceivedAtTimestamp, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) return attachmentStore.fetchReferences( owners: [.messageBodyAttachment(messageRowId: messageId)], - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ).first! } @@ -570,7 +537,6 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.insert( attachmentParams, reference: attachmentReferenceParams, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) } @@ -579,12 +545,10 @@ class AttachmentStoreTests: XCTestCase { return db.read { tx in let references = attachmentStore.fetchReferences( owners: [.messageBodyAttachment(messageRowId: messageId)], - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) return attachmentStore.fetch( ids: references.map(\.attachmentRowId), - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ).first! } @@ -614,7 +578,6 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.markUploadedToTransitTier( attachmentStream: stream, info: transitTierInfo, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) } @@ -641,7 +604,6 @@ class AttachmentStoreTests: XCTestCase { messageRowId: messageId1, threadRowId: threadId1 ), - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) let attachmentId = InMemoryDB.shimOnlyBridge(tx).db.lastInsertedRowID @@ -651,7 +613,6 @@ class AttachmentStoreTests: XCTestCase { threadRowId: threadId2 ), for: attachmentId, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) } @@ -659,12 +620,10 @@ class AttachmentStoreTests: XCTestCase { let (reference1, reference2) = db.read { tx in let reference1 = attachmentStore.fetchReferences( owners: [.messageBodyAttachment(messageRowId: messageId1)], - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ).first! let reference2 = attachmentStore.fetchReferences( owners: [.messageBodyAttachment(messageRowId: messageId2)], - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ).first! return (reference1, reference2) @@ -677,14 +636,12 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.removeOwner( reference1.owner.id, for: reference1.attachmentRowId, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) // The attachment should still exist. XCTAssertNotNil(attachmentStore.fetch( ids: [reference1.attachmentRowId], - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ).first) @@ -692,14 +649,12 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.removeOwner( reference2.owner.id, for: reference2.attachmentRowId, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) // The attachment should no longer exist. XCTAssertNil(attachmentStore.fetch( ids: [reference1.attachmentRowId], - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ).first) } @@ -727,7 +682,6 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.insert( attachmentParams, reference: referenceParams, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) } @@ -741,12 +695,10 @@ class AttachmentStoreTests: XCTestCase { .threadWallpaperImage(threadRowId: $0) } ?? .globalThreadWallpaperImage }, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) let attachments = attachmentStore.fetch( ids: references.map(\.attachmentRowId), - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) XCTAssertEqual(references.count, expectedCount) @@ -760,7 +712,6 @@ class AttachmentStoreTests: XCTestCase { try db.write { tx in try attachmentStore.removeAllThreadOwners( - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) } @@ -790,7 +741,6 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.insert( attachmentParams, reference: originalReferenceParams, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) } @@ -799,7 +749,6 @@ class AttachmentStoreTests: XCTestCase { let reference1 = db.read { tx in return attachmentStore.fetchReferences( owners: [.messageBodyAttachment(messageRowId: messageId1)], - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ).first! } @@ -813,7 +762,6 @@ class AttachmentStoreTests: XCTestCase { with: reference1, newOwnerMessageRowId: messageId2, newOwnerThreadRowId: threadId, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) default: @@ -825,7 +773,6 @@ class AttachmentStoreTests: XCTestCase { let newReference = db.read { tx in return attachmentStore.fetchReferences( owners: [.messageBodyAttachment(messageRowId: messageId2)], - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ).first! } @@ -847,7 +794,6 @@ class AttachmentStoreTests: XCTestCase { try attachmentStore.insert( attachmentParams, reference: originalReferenceParams, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) } @@ -856,7 +802,6 @@ class AttachmentStoreTests: XCTestCase { let reference1 = db.read { tx in return attachmentStore.fetchReferences( owners: [.messageBodyAttachment(messageRowId: messageId1)], - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ).first! } @@ -874,7 +819,6 @@ class AttachmentStoreTests: XCTestCase { with: reference1, newOwnerMessageRowId: messageId2, newOwnerThreadRowId: threadId2, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) default: diff --git a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadQueueDBTests.swift b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadQueueDBTests.swift index 2b4079ae28..a3c8e15b38 100644 --- a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadQueueDBTests.swift +++ b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadQueueDBTests.swift @@ -31,7 +31,6 @@ class AttachmentDownloadQueueDBTests: XCTestCase { try attachmentStore.insert( attachmentParams, reference: referenceParams, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) return try Int64.fetchOne( diff --git a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadStoreTests.swift b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadStoreTests.swift index 48bc6bfa75..b7d03578b4 100644 --- a/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadStoreTests.swift +++ b/SignalServiceKit/Messages/Attachments/V2/Downloads/AttachmentDownloadStoreTests.swift @@ -291,7 +291,6 @@ class AttachmentDownloadStoreTests: XCTestCase { try attachmentStore.insert( attachmentParams, reference: referenceParams, - db: tx.db, tx: tx ) return tx.db.lastInsertedRowID diff --git a/SignalServiceKit/Messages/Attachments/V2/OrphanedAttachments/OrphanedAttachmentCleanerTest.swift b/SignalServiceKit/Messages/Attachments/V2/OrphanedAttachments/OrphanedAttachmentCleanerTest.swift index 9832005d9d..344285e294 100644 --- a/SignalServiceKit/Messages/Attachments/V2/OrphanedAttachments/OrphanedAttachmentCleanerTest.swift +++ b/SignalServiceKit/Messages/Attachments/V2/OrphanedAttachments/OrphanedAttachmentCleanerTest.swift @@ -42,7 +42,6 @@ class OrphanedAttachmentCleanerTest: XCTestCase { try attachmentStore.insert( attachmentParams, reference: referenceParams, - db: InMemoryDB.shimOnlyBridge(tx).db, tx: tx ) }