diff --git a/Session/Shared/FullConversationCell.swift b/Session/Shared/FullConversationCell.swift index d645035d1..b131b808a 100644 --- a/Session/Shared/FullConversationCell.swift +++ b/Session/Shared/FullConversationCell.swift @@ -381,6 +381,9 @@ public final class FullConversationCell: UITableViewCell { // MARK: - Snippet generation private func getSnippet(cellViewModel: SessionThreadViewModel) -> NSMutableAttributedString { + // If we don't have an interaction then do nothing + guard cellViewModel.interactionId != nil else { return NSMutableAttributedString() } + let result = NSMutableAttributedString() if Date().timeIntervalSince1970 < (cellViewModel.threadMutedUntilTimestamp ?? 0) { diff --git a/SessionMessagingKit/Database/Models/Interaction.swift b/SessionMessagingKit/Database/Models/Interaction.swift index 43c63b5c1..82ea89024 100644 --- a/SessionMessagingKit/Database/Models/Interaction.swift +++ b/SessionMessagingKit/Database/Models/Interaction.swift @@ -462,7 +462,24 @@ public extension Interaction { } // If we aren't including older interactions then update and save the current one - guard includingOlder else { + struct InteractionReadInfo: Decodable, FetchableRecord { + let timestampMs: Int64 + let wasRead: Bool + } + + // Since there is no guarantee on the order messages are inserted into the database + // fetch the timestamp for the interaction and set everything before that as read + let maybeInteractionInfo: InteractionReadInfo? = try Interaction + .select(.timestampMs, .wasRead) + .filter(id: interactionId) + .asRequest(of: InteractionReadInfo.self) + .fetchOne(db) + + guard includingOlder, let interactionInfo: InteractionReadInfo = maybeInteractionInfo else { + // Only mark as read and trigger the subsequent jobs if the interaction is + // actually not read (no point updating and triggering db changes otherwise) + guard maybeInteractionInfo?.wasRead == false else { return } + _ = try Interaction .filter(id: interactionId) .updateAll(db, Columns.wasRead.set(to: true)) @@ -472,9 +489,9 @@ public extension Interaction { } let interactionQuery = Interaction - .filter(Columns.threadId == threadId) - .filter(Columns.id <= interactionId) - .filter(Columns.wasRead == false) + .filter(Interaction.Columns.threadId == threadId) + .filter(Interaction.Columns.timestampMs <= interactionInfo.timestampMs) + .filter(Interaction.Columns.wasRead == false) // The `wasRead` flag doesn't apply to `standardOutgoing` or `standardIncomingDeleted` .filter(Columns.variant != Variant.standardOutgoing && Columns.variant != Variant.standardIncomingDeleted) let interactionIdsToMarkAsRead: [Int64] = try interactionQuery diff --git a/SessionMessagingKit/Jobs/Types/GarbageCollectionJob.swift b/SessionMessagingKit/Jobs/Types/GarbageCollectionJob.swift index b5d73c972..42c8de672 100644 --- a/SessionMessagingKit/Jobs/Types/GarbageCollectionJob.swift +++ b/SessionMessagingKit/Jobs/Types/GarbageCollectionJob.swift @@ -35,8 +35,6 @@ public enum GarbageCollectionJob: JobExecutor { } let timestampNow: TimeInterval = Date().timeIntervalSince1970 - var attachmentLocalRelativePaths: Set = [] - var profileAvatarFilenames: Set = [] GRDBStorage.shared.writeAsync( updates: { db in @@ -203,109 +201,127 @@ public enum GarbageCollectionJob: JobExecutor { ) """) } - - /// Orphaned attachment files - attachment files which don't have an associated record in the database - if details.typesToCollect.contains(.orphanedAttachmentFiles) { - /// **Note:** Thumbnails are stored in the `NSCachesDirectory` directory which should be automatically manage - /// it's own garbage collection so we can just ignore it according to the various comments in the following stack overflow - /// post, the directory will be cleared during app updates as well as if the system is running low on memory (if the app isn't running) - /// https://stackoverflow.com/questions/6879860/when-are-files-from-nscachesdirectory-removed - attachmentLocalRelativePaths = try Attachment - .select(.localRelativeFilePath) - .filter(Attachment.Columns.localRelativeFilePath != nil) - .asRequest(of: String.self) - .fetchSet(db) - } - - /// Orphaned profile avatar files - profile avatar files which don't have an associated record in the database - if details.typesToCollect.contains(.orphanedProfileAvatars) { - profileAvatarFilenames = try Profile - .select(.profilePictureFileName) - .filter(Profile.Columns.profilePictureFileName != nil) - .asRequest(of: String.self) - .fetchSet(db) - } }, - completion: { _, result in - // If any of the above failed then we don't want to continue (we would end up deleting all files since - // neither of the arrays would have been populated correctly) - guard case .success = result else { - SNLog("[GarbageCollectionJob] Database queries failed, skipping file cleanup") - return - } - - var deletionErrors: [Error] = [] - - // Orphaned attachment files (actual deletion) - if details.typesToCollect.contains(.orphanedAttachmentFiles) { - // Note: Looks like in order to recursively look through files we need to use the - // enumerator method - let fileEnumerator = FileManager.default.enumerator( - at: URL(fileURLWithPath: Attachment.attachmentsFolder), - includingPropertiesForKeys: nil, - options: .skipsHiddenFiles // Ignore the `.DS_Store` for the simulator - ) + completion: { _, _ in + // Dispatch async so we can swap from the write queue to a read one (we are done writing) + queue.async { + // Retrieve a list of all valid attachmnet and avatar file paths + struct FileInfo { + let attachmentLocalRelativePaths: Set + let profileAvatarFilenames: Set + } - let allAttachmentFilePaths: Set = (fileEnumerator? - .allObjects - .compactMap { Attachment.localRelativeFilePath(from: ($0 as? URL)?.path) }) - .defaulting(to: []) - .asSet() + let maybeFileInfo: FileInfo? = GRDBStorage.shared.read { db -> FileInfo in + var attachmentLocalRelativePaths: Set = [] + var profileAvatarFilenames: Set = [] + + /// Orphaned attachment files - attachment files which don't have an associated record in the database + if details.typesToCollect.contains(.orphanedAttachmentFiles) { + /// **Note:** Thumbnails are stored in the `NSCachesDirectory` directory which should be automatically manage + /// it's own garbage collection so we can just ignore it according to the various comments in the following stack overflow + /// post, the directory will be cleared during app updates as well as if the system is running low on memory (if the app isn't running) + /// https://stackoverflow.com/questions/6879860/when-are-files-from-nscachesdirectory-removed + attachmentLocalRelativePaths = try Attachment + .select(.localRelativeFilePath) + .filter(Attachment.Columns.localRelativeFilePath != nil) + .asRequest(of: String.self) + .fetchSet(db) + } + + /// Orphaned profile avatar files - profile avatar files which don't have an associated record in the database + if details.typesToCollect.contains(.orphanedProfileAvatars) { + profileAvatarFilenames = try Profile + .select(.profilePictureFileName) + .filter(Profile.Columns.profilePictureFileName != nil) + .asRequest(of: String.self) + .fetchSet(db) + } + + return FileInfo( + attachmentLocalRelativePaths: attachmentLocalRelativePaths, + profileAvatarFilenames: profileAvatarFilenames + ) + } - // Note: Directories will have their own entries in the list, if there is a folder with content - // the file will include the directory in it's path with a forward slash so we can use this to - // distinguish empty directories from ones with content so we don't unintentionally delete a - // directory which contains content to keep as well as delete (directories which end up empty after - // this clean up will be removed during the next run) - let directoryNamesContainingContent: [String] = allAttachmentFilePaths - .filter { path -> Bool in path.contains("/") } - .compactMap { path -> String? in path.components(separatedBy: "/").first } - let orphanedAttachmentFiles: Set = allAttachmentFilePaths - .subtracting(attachmentLocalRelativePaths) - .subtracting(directoryNamesContainingContent) + // If we couldn't get the file lists then fail (invalid state and don't want to delete all attachment/profile files) + guard let fileInfo: FileInfo = maybeFileInfo else { + failure(job, StorageError.generic, false) + return + } + + var deletionErrors: [Error] = [] - orphanedAttachmentFiles.forEach { filepath in - // We don't want a single deletion failure to block deletion of the other files so try - // each one and store the error to be used to determine success/failure of the job - do { - try FileManager.default.removeItem( - atPath: URL(fileURLWithPath: Attachment.attachmentsFolder) - .appendingPathComponent(filepath) - .path - ) + // Orphaned attachment files (actual deletion) + if details.typesToCollect.contains(.orphanedAttachmentFiles) { + // Note: Looks like in order to recursively look through files we need to use the + // enumerator method + let fileEnumerator = FileManager.default.enumerator( + at: URL(fileURLWithPath: Attachment.attachmentsFolder), + includingPropertiesForKeys: nil, + options: .skipsHiddenFiles // Ignore the `.DS_Store` for the simulator + ) + + let allAttachmentFilePaths: Set = (fileEnumerator? + .allObjects + .compactMap { Attachment.localRelativeFilePath(from: ($0 as? URL)?.path) }) + .defaulting(to: []) + .asSet() + + // Note: Directories will have their own entries in the list, if there is a folder with content + // the file will include the directory in it's path with a forward slash so we can use this to + // distinguish empty directories from ones with content so we don't unintentionally delete a + // directory which contains content to keep as well as delete (directories which end up empty after + // this clean up will be removed during the next run) + let directoryNamesContainingContent: [String] = allAttachmentFilePaths + .filter { path -> Bool in path.contains("/") } + .compactMap { path -> String? in path.components(separatedBy: "/").first } + let orphanedAttachmentFiles: Set = allAttachmentFilePaths + .subtracting(fileInfo.attachmentLocalRelativePaths) + .subtracting(directoryNamesContainingContent) + + orphanedAttachmentFiles.forEach { filepath in + // We don't want a single deletion failure to block deletion of the other files so try + // each one and store the error to be used to determine success/failure of the job + do { + try FileManager.default.removeItem( + atPath: URL(fileURLWithPath: Attachment.attachmentsFolder) + .appendingPathComponent(filepath) + .path + ) + } + catch { deletionErrors.append(error) } } - catch { deletionErrors.append(error) } } - } - - // Orphaned profile avatar files (actual deletion) - if details.typesToCollect.contains(.orphanedProfileAvatars) { - let allAvatarProfileFilenames: Set = (try? FileManager.default - .contentsOfDirectory(atPath: ProfileManager.sharedDataProfileAvatarsDirPath)) - .defaulting(to: []) - .asSet() - let orphanedAvatarFiles: Set = allAvatarProfileFilenames - .subtracting(profileAvatarFilenames) - orphanedAvatarFiles.forEach { filename in - // We don't want a single deletion failure to block deletion of the other files so try - // each one and store the error to be used to determine success/failure of the job - do { - try FileManager.default.removeItem( - atPath: ProfileManager.profileAvatarFilepath(filename: filename) - ) + // Orphaned profile avatar files (actual deletion) + if details.typesToCollect.contains(.orphanedProfileAvatars) { + let allAvatarProfileFilenames: Set = (try? FileManager.default + .contentsOfDirectory(atPath: ProfileManager.sharedDataProfileAvatarsDirPath)) + .defaulting(to: []) + .asSet() + let orphanedAvatarFiles: Set = allAvatarProfileFilenames + .subtracting(fileInfo.profileAvatarFilenames) + + orphanedAvatarFiles.forEach { filename in + // We don't want a single deletion failure to block deletion of the other files so try + // each one and store the error to be used to determine success/failure of the job + do { + try FileManager.default.removeItem( + atPath: ProfileManager.profileAvatarFilepath(filename: filename) + ) + } + catch { deletionErrors.append(error) } } - catch { deletionErrors.append(error) } } + + // Report a single file deletion as a job failure (even if other content was successfully removed) + guard deletionErrors.isEmpty else { + failure(job, (deletionErrors.first ?? StorageError.generic), false) + return + } + + success(job, false) } - - // Report a single file deletion as a job failure (even if other content was successfully removed) - guard deletionErrors.isEmpty else { - failure(job, (deletionErrors.first ?? StorageError.generic), false) - return - } - - success(job, false) } ) } diff --git a/SessionMessagingKit/Open Groups/OpenGroupManager.swift b/SessionMessagingKit/Open Groups/OpenGroupManager.swift index c3f60f60b..602159fcf 100644 --- a/SessionMessagingKit/Open Groups/OpenGroupManager.swift +++ b/SessionMessagingKit/Open Groups/OpenGroupManager.swift @@ -169,6 +169,7 @@ public final class OpenGroupManager: NSObject { // Optionally try to insert a new version of the OpenGroup (it will fail if there is already an // inactive one but that won't matter as we then activate it _ = try? SessionThread.fetchOrCreate(db, id: threadId, variant: .openGroup) + _ = try? SessionThread.filter(id: threadId).updateAll(db, SessionThread.Columns.shouldBeVisible.set(to: true)) if (try? OpenGroup.exists(db, id: threadId)) == false { try? OpenGroup diff --git a/SessionMessagingKit/Shared Models/MessageViewModel.swift b/SessionMessagingKit/Shared Models/MessageViewModel.swift index 34193045f..67f37827a 100644 --- a/SessionMessagingKit/Shared Models/MessageViewModel.swift +++ b/SessionMessagingKit/Shared Models/MessageViewModel.swift @@ -101,6 +101,8 @@ public struct MessageViewModel: FetchableRecordWithRowId, Decodable, Equatable, public let authorName: String /// This value will be used to populate the author label, if it's null then the label will be hidden + /// + /// **Note:** This will only be populated for incoming messages public let senderName: String? /// A flag indicating whether the profile view should be displayed @@ -330,6 +332,11 @@ public struct MessageViewModel: FetchableRecordWithRowId, Decodable, Equatable, guard self.threadVariant == .openGroup || self.threadVariant == .closedGroup else { return nil } + + // Only show for incoming messages + guard self.variant == .standardIncoming || self.variant == .standardIncomingDeleted else { + return nil + } // Only if there is a date header or the senders are different guard shouldShowDateOnThisModel || self.authorId != prevModel?.authorId else { diff --git a/SessionMessagingKit/Shared Models/SessionThreadViewModel.swift b/SessionMessagingKit/Shared Models/SessionThreadViewModel.swift index 2b3a085cd..a4401df7f 100644 --- a/SessionMessagingKit/Shared Models/SessionThreadViewModel.swift +++ b/SessionMessagingKit/Shared Models/SessionThreadViewModel.swift @@ -500,13 +500,13 @@ public extension SessionThreadViewModel { static let homeOrderSQL: SQL = { let thread: TypedTableAlias = TypedTableAlias() - return SQL("\(thread[.isPinned]) DESC, IFNULL(\(Interaction.self).\(ViewModel.interactionTimestampMsKey), \(thread[.creationDateTimestamp])) DESC") + return SQL("\(thread[.isPinned]) DESC, IFNULL(\(Interaction.self).\(ViewModel.interactionTimestampMsKey), (\(thread[.creationDateTimestamp]) * 1000)) DESC") }() static let messageRequetsOrderSQL: SQL = { let thread: TypedTableAlias = TypedTableAlias() - return SQL("IFNULL(\(Interaction.self).\(ViewModel.interactionTimestampMsKey), \(thread[.creationDateTimestamp])) DESC") + return SQL("IFNULL(\(Interaction.self).\(ViewModel.interactionTimestampMsKey), (\(thread[.creationDateTimestamp]) * 1000)) DESC") }() } @@ -1388,7 +1388,7 @@ public extension SessionThreadViewModel { ) GROUP BY \(thread[.id]) - ORDER BY IFNULL(\(interaction[.timestampMs]), \(thread[.creationDateTimestamp])) DESC + ORDER BY IFNULL(\(interaction[.timestampMs]), (\(thread[.creationDateTimestamp]) * 1000)) DESC """ return request.adapted { db in