Fixed a number of bugs

Fixed a bug where threads might not be getting marked as read correctly
Fixed a bug where the GarbageCollectionJob could end up blocking the database write thread (seemed to only hang when the debugger was attached but may have affected devices at some point)
Fixed a bug with thread sorting
Fixed a bug where joining an open group wouldn't appear until after the first poll completed
Fixed a bug where conversations with no interactions would display odd interaction copy
Fixed a bug where the sender name was appearing above outgoing messages in groups
pull/612/head
Morgan Pretty 3 years ago
parent 20dc74bc96
commit 2cd9f571da

@ -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) {

@ -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

@ -35,8 +35,6 @@ public enum GarbageCollectionJob: JobExecutor {
}
let timestampNow: TimeInterval = Date().timeIntervalSince1970
var attachmentLocalRelativePaths: Set<String> = []
var profileAvatarFilenames: Set<String> = []
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<String>
let profileAvatarFilenames: Set<String>
}
let allAttachmentFilePaths: Set<String> = (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<String> = []
var profileAvatarFilenames: Set<String> = []
/// 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<String> = 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<String> = (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<String> = 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<String> = (try? FileManager.default
.contentsOfDirectory(atPath: ProfileManager.sharedDataProfileAvatarsDirPath))
.defaulting(to: [])
.asSet()
let orphanedAvatarFiles: Set<String> = 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<String> = (try? FileManager.default
.contentsOfDirectory(atPath: ProfileManager.sharedDataProfileAvatarsDirPath))
.defaulting(to: [])
.asSet()
let orphanedAvatarFiles: Set<String> = 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)
}
)
}

@ -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

@ -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 {

@ -500,13 +500,13 @@ public extension SessionThreadViewModel {
static let homeOrderSQL: SQL = {
let thread: TypedTableAlias<SessionThread> = 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<SessionThread> = 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

Loading…
Cancel
Save