From 93b54a3b7d1a518feea33ab7f7e6a26e08d3f52f Mon Sep 17 00:00:00 2001 From: Morgan Pretty <morgan.t.pretty@gmail.com> Date: Thu, 2 Jun 2022 14:13:07 +1000 Subject: [PATCH] Added logic for the GarbageCollectionJob Fixed a bug where the GalleryRailView wasn't appearing Fixed a database reentrancy error Fixed a bug where the threadId for migrated attachmentUpload jobs wasn't getting set to the non-legacy version Deleted the buggy overwritten 'delete' methods to avoid confusion Updated the GroupMember table to cascade delete if it's thread is deleted (can't do so for the Closed/Open group but they share the same id conveniently) Updated the 'isRunningTests' to be based on the existence of XCInjectBundleInfo which seems to be more consistent than the old approach --- Session/Meta/MainAppContext.m | 2 +- .../_001_InitialSetupMigration.swift | 8 +- .../Migrations/_002_SetupStandardJobs.swift | 7 +- .../Migrations/_003_YDBToGRDBMigration.swift | 4 +- .../Database/Models/Attachment.swift | 57 +++-- .../Database/Models/ClosedGroup.swift | 10 - .../Database/Models/Interaction.swift | 27 --- .../Models/InteractionAttachment.swift | 21 -- .../Database/Models/LinkPreview.swift | 21 -- .../Database/Models/OpenGroup.swift | 10 - .../Database/Models/Quote.swift | 21 -- .../Database/Models/SessionThread.swift | 16 -- .../Jobs/Types/AttachmentUploadJob.swift | 30 +-- .../Jobs/Types/GarbageCollectionJob.swift | 218 +++++++++++++++++- .../Utilities/ProfileManager.swift | 2 +- .../Database/GRDBStorage.swift | 9 - SessionUtilitiesKit/JobRunner/JobRunner.swift | 7 +- .../Shared Views/GalleryRailView.swift | 7 +- 18 files changed, 280 insertions(+), 197 deletions(-) diff --git a/Session/Meta/MainAppContext.m b/Session/Meta/MainAppContext.m index bfcf801a2..704bf481d 100644 --- a/Session/Meta/MainAppContext.m +++ b/Session/Meta/MainAppContext.m @@ -245,7 +245,7 @@ NSString *const ReportedApplicationStateDidChangeNotification = @"ReportedApplic - (BOOL)isRunningTests { - return getenv("runningTests_dontStartApp"); + return (NSProcessInfo.processInfo.environment[@"XCInjectBundleInto"] != nil); } - (void)setNetworkActivityIndicatorVisible:(BOOL)value diff --git a/SessionMessagingKit/Database/Migrations/_001_InitialSetupMigration.swift b/SessionMessagingKit/Database/Migrations/_001_InitialSetupMigration.swift index 345068b11..8084cfcbd 100644 --- a/SessionMessagingKit/Database/Migrations/_001_InitialSetupMigration.swift +++ b/SessionMessagingKit/Database/Migrations/_001_InitialSetupMigration.swift @@ -152,11 +152,13 @@ enum _001_InitialSetupMigration: Migration { } try db.create(table: GroupMember.self) { t in - // Note: Not adding a "proper" foreign key constraint as this - // table gets used by both 'OpenGroup' and 'ClosedGroup' types + // Note: Since we don't know whether this will be stored against a 'ClosedGroup' or + // an 'OpenGroup' we add the foreign key constraint against the thread itself (which + // shares the same 'id' as the 'groupId') so we can cascade delete automatically t.column(.groupId, .text) .notNull() .indexed() // Quicker querying + .references(SessionThread.self, onDelete: .cascade) // Delete if Thread deleted t.column(.profileId, .text).notNull() t.column(.role, .integer).notNull() } @@ -316,7 +318,7 @@ enum _001_InitialSetupMigration: Migration { t.column(.variant, .integer).notNull() t.column(.title, .text) t.column(.attachmentId, .text) - .references(Attachment.self, onDelete: .setNull) // Clear if attachment deleted + .references(Attachment.self) // Managed via garbage collection t.primaryKey([.url, .timestamp]) } diff --git a/SessionMessagingKit/Database/Migrations/_002_SetupStandardJobs.swift b/SessionMessagingKit/Database/Migrations/_002_SetupStandardJobs.swift index 877693bf5..005506099 100644 --- a/SessionMessagingKit/Database/Migrations/_002_SetupStandardJobs.swift +++ b/SessionMessagingKit/Database/Migrations/_002_SetupStandardJobs.swift @@ -48,8 +48,11 @@ enum _002_SetupStandardJobs: Migration { _ = try Job( variant: .garbageCollection, - behaviour: .recurringOnLaunch - ).inserted(db) + behaviour: .recurringOnLaunch, + details: GarbageCollectionJob.Details( + typesToCollect: GarbageCollectionJob.Types.allCases + ) + )?.inserted(db) } GRDBStorage.shared.update(progress: 1, for: self, in: target) // In case this is the last migration diff --git a/SessionMessagingKit/Database/Migrations/_003_YDBToGRDBMigration.swift b/SessionMessagingKit/Database/Migrations/_003_YDBToGRDBMigration.swift index 9497d4ce1..2b8cac9f7 100644 --- a/SessionMessagingKit/Database/Migrations/_003_YDBToGRDBMigration.swift +++ b/SessionMessagingKit/Database/Migrations/_003_YDBToGRDBMigration.swift @@ -1207,12 +1207,12 @@ enum _003_YDBToGRDBMigration: Migration { SNLog("[Migration Error] attachmentUpload job missing associated MessageSendJob") throw StorageError.migrationFailed } - + let uploadJob: Job? = try Job( failureCount: legacyJob.failureCount, variant: .attachmentUpload, behaviour: .runOnce, - threadId: legacyJob.threadID, + threadId: sendJob.threadId, interactionId: sendJob.interactionId, details: AttachmentUploadJob.Details( messageSendJobId: sendJobId, diff --git a/SessionMessagingKit/Database/Models/Attachment.swift b/SessionMessagingKit/Database/Models/Attachment.swift index 43700cc79..e8b08a686 100644 --- a/SessionMessagingKit/Database/Models/Attachment.swift +++ b/SessionMessagingKit/Database/Models/Attachment.swift @@ -219,28 +219,6 @@ public struct Attachment: Codable, Identifiable, Equatable, Hashable, FetchableR self.digest = nil self.caption = caption } - - // MARK: - Custom Database Interaction - - public func delete(_ db: Database) throws -> Bool { - // Delete all associated files - if FileManager.default.fileExists(atPath: thumbnailsDirPath) { - try? FileManager.default.removeItem(atPath: thumbnailsDirPath) - } - - if - let legacyThumbnailPath: String = legacyThumbnailPath, - FileManager.default.fileExists(atPath: legacyThumbnailPath) - { - try? FileManager.default.removeItem(atPath: legacyThumbnailPath) - } - - if let originalFilePath: String = originalFilePath { - try? FileManager.default.removeItem(atPath: originalFilePath) - } - - return try performDelete(db) - } } // MARK: - CustomStringConvertible @@ -941,7 +919,7 @@ extension Attachment { extension Attachment { internal func upload( - _ db: Database, + _ db: Database? = nil, using upload: (Data) -> Promise<UInt64>, encrypt: Bool, success: (() -> Void)?, @@ -977,9 +955,19 @@ extension Attachment { digest == nil else { // Save the final upload info - let uploadedAttachment: Attachment? = try? self - .with(state: .uploaded) - .saved(db) + let uploadedAttachment: Attachment? = { + guard let db: Database = db else { + return GRDBStorage.shared.write { db in + try? self + .with(state: .uploaded) + .saved(db) + } + } + + return try? self + .with(state: .uploaded) + .saved(db) + }() guard uploadedAttachment != nil else { SNLog("Couldn't update attachmentUpload job.") @@ -1019,9 +1007,19 @@ extension Attachment { } // Update the attachment to the 'uploading' state - let updatedAttachment: Attachment? = try? processedAttachment - .with(state: .uploading) - .saved(db) + let updatedAttachment: Attachment? = { + guard let db: Database = db else { + return GRDBStorage.shared.write { db in + try? processedAttachment + .with(state: .uploading) + .saved(db) + } + } + + return try? processedAttachment + .with(state: .uploading) + .saved(db) + }() guard updatedAttachment != nil else { SNLog("Couldn't update attachmentUpload job.") @@ -1061,6 +1059,7 @@ extension Attachment { .with(state: .failedUpload) .saved(db) } + failure?(error) } } diff --git a/SessionMessagingKit/Database/Models/ClosedGroup.swift b/SessionMessagingKit/Database/Models/ClosedGroup.swift index d9c087310..dcd4b9e81 100644 --- a/SessionMessagingKit/Database/Models/ClosedGroup.swift +++ b/SessionMessagingKit/Database/Models/ClosedGroup.swift @@ -76,16 +76,6 @@ public struct ClosedGroup: Codable, Identifiable, FetchableRecord, PersistableRe self.name = name self.formationTimestamp = formationTimestamp } - - // MARK: - Custom Database Interaction - - public func delete(_ db: Database) throws -> Bool { - // Delete all 'GroupMember' records associated with this ClosedGroup (can't - // have a proper ForeignKey constraint as 'GroupMember' is reused for the - // 'OpenGroup' table as well) - try request(for: ClosedGroup.members).deleteAll(db) - return try performDelete(db) - } } // MARK: - Mutation diff --git a/SessionMessagingKit/Database/Models/Interaction.swift b/SessionMessagingKit/Database/Models/Interaction.swift index 12160586a..bef5e5226 100644 --- a/SessionMessagingKit/Database/Models/Interaction.swift +++ b/SessionMessagingKit/Database/Models/Interaction.swift @@ -345,33 +345,6 @@ public struct Interaction: Codable, Identifiable, Equatable, FetchableRecord, Mu default: break } } - - public func delete(_ db: Database) throws -> Bool { - // If we have a LinkPreview then check if this is the only interaction that has it - // and delete the LinkPreview if so - if linkPreviewUrl != nil { - let interactionAlias: TableAlias = TableAlias() - let numInteractions: Int? = try? Interaction - .aliased(interactionAlias) - .joining( - required: Interaction.linkPreview - .filter(literal: Interaction.linkPreviewFilterLiteral()) - ) - .fetchCount(db) - let tmp = try linkPreview.fetchAll(db) - - if numInteractions == 1 { - try linkPreview.deleteAll(db) - } - } - - // Delete any jobs associated to this interaction - try Job - .filter(Job.Columns.interactionId == id) - .deleteAll(db) - - return try performDelete(db) - } } // MARK: - Mutation diff --git a/SessionMessagingKit/Database/Models/InteractionAttachment.swift b/SessionMessagingKit/Database/Models/InteractionAttachment.swift index 465c124b2..05feb2132 100644 --- a/SessionMessagingKit/Database/Models/InteractionAttachment.swift +++ b/SessionMessagingKit/Database/Models/InteractionAttachment.swift @@ -43,25 +43,4 @@ public struct InteractionAttachment: Codable, Equatable, FetchableRecord, Persis self.interactionId = interactionId self.attachmentId = attachmentId } - - // MARK: - Custom Database Interaction - - public func delete(_ db: Database) throws -> Bool { - // If we have an Attachment then check if this is the only type that is referencing it - // and delete the Attachment if so - let quoteUses: Int? = try? Quote - .select(.attachmentId) - .filter(Quote.Columns.attachmentId == attachmentId) - .fetchCount(db) - let linkPreviewUses: Int? = try? LinkPreview - .select(.attachmentId) - .filter(LinkPreview.Columns.attachmentId == attachmentId) - .fetchCount(db) - - if (quoteUses ?? 0) == 0 && (linkPreviewUses ?? 0) == 0 { - try attachment.deleteAll(db) - } - - return try performDelete(db) - } } diff --git a/SessionMessagingKit/Database/Models/LinkPreview.swift b/SessionMessagingKit/Database/Models/LinkPreview.swift index 9fe7eaa0e..03cb18f66 100644 --- a/SessionMessagingKit/Database/Models/LinkPreview.swift +++ b/SessionMessagingKit/Database/Models/LinkPreview.swift @@ -72,27 +72,6 @@ public struct LinkPreview: Codable, Equatable, Hashable, FetchableRecord, Persis self.title = title self.attachmentId = attachmentId } - - // MARK: - Custom Database Interaction - - public func delete(_ db: Database) throws -> Bool { - // If we have an Attachment then check if this is the only type that is referencing it - // and delete the Attachment if so - if let attachmentId: String = attachmentId { - let interactionUses: Int? = try? InteractionAttachment - .filter(InteractionAttachment.Columns.attachmentId == attachmentId) - .fetchCount(db) - let quoteUses: Int? = try? Quote - .filter(Quote.Columns.attachmentId == attachmentId) - .fetchCount(db) - - if (interactionUses ?? 0) == 0 && (quoteUses ?? 0) == 0 { - try attachment.deleteAll(db) - } - } - - return try performDelete(db) - } } // MARK: - Protobuf diff --git a/SessionMessagingKit/Database/Models/OpenGroup.swift b/SessionMessagingKit/Database/Models/OpenGroup.swift index ddb12ab03..c85aa7dea 100644 --- a/SessionMessagingKit/Database/Models/OpenGroup.swift +++ b/SessionMessagingKit/Database/Models/OpenGroup.swift @@ -104,16 +104,6 @@ public struct OpenGroup: Codable, Identifiable, FetchableRecord, PersistableReco self.userCount = userCount self.infoUpdates = infoUpdates } - - // MARK: - Custom Database Interaction - - public func delete(_ db: Database) throws -> Bool { - // Delete all 'GroupMember' records associated with this OpenGroup (can't - // have a proper ForeignKey constraint as 'GroupMember' is reused for the - // 'ClosedGroup' table as well) - try request(for: OpenGroup.members).deleteAll(db) - return try performDelete(db) - } } // MARK: - Convenience diff --git a/SessionMessagingKit/Database/Models/Quote.swift b/SessionMessagingKit/Database/Models/Quote.swift index 5a867f1de..9ac83853c 100644 --- a/SessionMessagingKit/Database/Models/Quote.swift +++ b/SessionMessagingKit/Database/Models/Quote.swift @@ -74,27 +74,6 @@ public struct Quote: Codable, Equatable, Hashable, FetchableRecord, PersistableR self.body = body self.attachmentId = attachmentId } - - // MARK: - Custom Database Interaction - - public func delete(_ db: Database) throws -> Bool { - // If we have an Attachment then check if this is the only type that is referencing it - // and delete the Attachment if so - if let attachmentId: String = attachmentId { - let interactionUses: Int? = try? InteractionAttachment - .filter(InteractionAttachment.Columns.attachmentId == attachmentId) - .fetchCount(db) - let linkPreviewUses: Int? = try? LinkPreview - .filter(LinkPreview.Columns.attachmentId == attachmentId) - .fetchCount(db) - - if (interactionUses ?? 0) == 0 && (linkPreviewUses ?? 0) == 0 { - try attachment.deleteAll(db) - } - } - - return try performDelete(db) - } } // MARK: - Protobuf diff --git a/SessionMessagingKit/Database/Models/SessionThread.swift b/SessionMessagingKit/Database/Models/SessionThread.swift index 803fe7f22..b37e0fe9f 100644 --- a/SessionMessagingKit/Database/Models/SessionThread.swift +++ b/SessionMessagingKit/Database/Models/SessionThread.swift @@ -129,22 +129,6 @@ public struct SessionThread: Codable, Identifiable, Equatable, FetchableRecord, db[.hasSavedThread] = true } - - public func delete(_ db: Database) throws -> Bool { - // Delete any jobs associated to this thread - try Job - .filter(Job.Columns.threadId == id) - .deleteAll(db) - - // Delete any GroupMembers associated to this thread - if variant == .closedGroup || variant == .openGroup { - try GroupMember - .filter(GroupMember.Columns.groupId == id) - .deleteAll(db) - } - - return try performDelete(db) - } } // MARK: - Mutation diff --git a/SessionMessagingKit/Jobs/Types/AttachmentUploadJob.swift b/SessionMessagingKit/Jobs/Types/AttachmentUploadJob.swift index f9965ba89..5d9eb1f82 100644 --- a/SessionMessagingKit/Jobs/Types/AttachmentUploadJob.swift +++ b/SessionMessagingKit/Jobs/Types/AttachmentUploadJob.swift @@ -33,21 +33,21 @@ public enum AttachmentUploadJob: JobExecutor { return } - GRDBStorage.shared.writeAsync { db in - attachment.upload( - db, - using: { data in - if let openGroup: OpenGroup = openGroup { - return OpenGroupAPIV2.upload(data, to: openGroup.room, on: openGroup.server) - } - - return FileServerAPIV2.upload(data) - }, - encrypt: (openGroup == nil), - success: { success(job, false) }, - failure: { error in failure(job, error, false) } - ) - } + // Note: In the AttachmentUploadJob we intentionally don't provide our own db instance to prevent reentrancy + // issues when the success/failure closures get called before the upload as the JobRunner will attempt to + // update the state of the job immediately + attachment.upload( + using: { data in + if let openGroup: OpenGroup = openGroup { + return OpenGroupAPIV2.upload(data, to: openGroup.room, on: openGroup.server) + } + + return FileServerAPIV2.upload(data) + }, + encrypt: (openGroup == nil), + success: { success(job, false) }, + failure: { error in failure(job, error, false) } + ) } } diff --git a/SessionMessagingKit/Jobs/Types/GarbageCollectionJob.swift b/SessionMessagingKit/Jobs/Types/GarbageCollectionJob.swift index 0d9bd516f..551f7d7fd 100644 --- a/SessionMessagingKit/Jobs/Types/GarbageCollectionJob.swift +++ b/SessionMessagingKit/Jobs/Types/GarbageCollectionJob.swift @@ -11,6 +11,7 @@ public enum GarbageCollectionJob: JobExecutor { public static var maxFailureCount: Int = -1 public static var requiresThreadId: Bool = false public static let requiresInteractionId: Bool = false + private static let approxSixMonthsInSeconds: TimeInterval = (6 * 30 * 24 * 60 * 60) public static func run( _ job: Job, @@ -26,7 +27,216 @@ public enum GarbageCollectionJob: JobExecutor { return } - failure(job, JobRunnerError.missingRequiredDetails, true) + // If there are no types to collect then complete the job (and never run again - it doesn't do anything) + guard !details.typesToCollect.isEmpty else { + success(job, true) + return + } + + let timestampNow: TimeInterval = Date().timeIntervalSince1970 + var attachmentLocalRelativePaths: Set<String> = [] + var profileAvatarFilenames: Set<String> = [] + + GRDBStorage.shared.writeAsync( + updates: { db in + // Remove any expired controlMessageProcessRecords + if details.typesToCollect.contains(.expiredControlMessageProcessRecords) { + _ = try ControlMessageProcessRecord + .filter(ControlMessageProcessRecord.Columns.serverExpirationTimestamp <= timestampNow) + .deleteAll(db) + } + + // Remove any typing indicators + if details.typesToCollect.contains(.threadTypingIndicators) { + _ = try ThreadTypingIndicator + .deleteAll(db) + } + + // Remove any typing indicators + if details.typesToCollect.contains(.oldOpenGroupMessages) { + let interaction: TypedTableAlias<Interaction> = TypedTableAlias() + let thread: TypedTableAlias<SessionThread> = TypedTableAlias() + + try db.execute(literal: """ + DELETE FROM \(Interaction.self) + WHERE \(Column.rowID) IN ( + SELECT \(interaction.alias[Column.rowID]) + FROM \(Interaction.self) + JOIN \(SessionThread.self) ON ( + \(SQL("\(thread[.variant]) = \(SessionThread.Variant.openGroup)")) AND + \(thread[.id]) = \(interaction[.threadId]) + ) + WHERE \(interaction[.timestampMs]) < \(timestampNow - approxSixMonthsInSeconds) + ) + """) + } + + // Orphaned jobs + if details.typesToCollect.contains(.orphanedJobs) { + let job: TypedTableAlias<Job> = TypedTableAlias() + let thread: TypedTableAlias<SessionThread> = TypedTableAlias() + let interaction: TypedTableAlias<Interaction> = TypedTableAlias() + + try db.execute(literal: """ + DELETE FROM \(Job.self) + WHERE \(Column.rowID) IN ( + SELECT \(job.alias[Column.rowID]) + FROM \(Job.self) + LEFT JOIN \(SessionThread.self) ON \(thread[.id]) = \(job[.threadId]) + LEFT JOIN \(Interaction.self) ON \(interaction[.id]) = \(job[.interactionId]) + WHERE ( + ( + \(job[.threadId]) IS NOT NULL AND + \(thread[.id]) IS NULL + ) OR ( + \(job[.interactionId]) IS NOT NULL AND + \(interaction[.id]) IS NULL + ) + ) + ) + """) + } + + // Orphaned link previews + if details.typesToCollect.contains(.orphanedLinkPreviews) { + let linkPreview: TypedTableAlias<LinkPreview> = TypedTableAlias() + let interaction: TypedTableAlias<Interaction> = TypedTableAlias() + + try db.execute(literal: """ + DELETE FROM \(LinkPreview.self) + WHERE \(Column.rowID) IN ( + SELECT \(linkPreview.alias[Column.rowID]) + FROM \(LinkPreview.self) + LEFT JOIN \(Interaction.self) ON ( + \(interaction[.linkPreviewUrl]) = \(linkPreview[.url]) AND + \(Interaction.linkPreviewFilterLiteral()) + ) + WHERE \(interaction[.id]) IS NULL + ) + """) + } + + // Orphaned attachments + if details.typesToCollect.contains(.orphanedAttachments) { + let attachment: TypedTableAlias<Attachment> = TypedTableAlias() + let quote: TypedTableAlias<Quote> = TypedTableAlias() + let linkPreview: TypedTableAlias<LinkPreview> = TypedTableAlias() + let interactionAttachment: TypedTableAlias<InteractionAttachment> = TypedTableAlias() + + try db.execute(literal: """ + DELETE FROM \(Attachment.self) + WHERE \(Column.rowID) IN ( + SELECT \(attachment.alias[Column.rowID]) + FROM \(Attachment.self) + LEFT JOIN \(Quote.self) ON \(quote[.attachmentId]) = \(attachment[.id]) + LEFT JOIN \(LinkPreview.self) ON \(linkPreview[.attachmentId]) = \(attachment[.id]) + LEFT JOIN \(InteractionAttachment.self) ON \(interactionAttachment[.attachmentId]) = \(attachment[.id]) + WHERE ( + \(quote[.attachmentId]) IS NULL AND + \(linkPreview[.url]) IS NULL AND + \(interactionAttachment[.attachmentId]) IS NULL + ) + ) + """) + } + + // Orphaned attachment files + 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 + if details.typesToCollect.contains(.orphanedProfileAvatars) { + profileAvatarFilenames = try Profile + .select(.profilePictureFileName) + .filter(Profile.Columns.profilePictureFileName != nil) + .asRequest(of: String.self) + .fetchSet(db) + } + }, + completion: { _, _ in + 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 + ) + + 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(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) } + } + } + + // 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) + ) + } + 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) + } + ) } } @@ -34,12 +244,14 @@ public enum GarbageCollectionJob: JobExecutor { extension GarbageCollectionJob { public enum Types: Codable, CaseIterable { - case oldOpenGroupMessages case expiredControlMessageProcessRecords case threadTypingIndicators + case oldOpenGroupMessages + case orphanedJobs + case orphanedLinkPreviews + case orphanedAttachments case orphanedAttachmentFiles case orphanedProfileAvatars - case orphanedLinkPreviews } public struct Details: Codable { diff --git a/SessionMessagingKit/Utilities/ProfileManager.swift b/SessionMessagingKit/Utilities/ProfileManager.swift index edcdb3f52..809e334ae 100644 --- a/SessionMessagingKit/Utilities/ProfileManager.swift +++ b/SessionMessagingKit/Utilities/ProfileManager.swift @@ -82,7 +82,7 @@ public struct ProfileManager { // MARK: - File Paths - private static let sharedDataProfileAvatarsDirPath: String = { + public static let sharedDataProfileAvatarsDirPath: String = { URL(fileURLWithPath: OWSFileSystem.appSharedDataDirectoryPath()) .appendingPathComponent("ProfileAvatars") .path diff --git a/SessionUtilitiesKit/Database/GRDBStorage.swift b/SessionUtilitiesKit/Database/GRDBStorage.swift index ff3326221..66d8cdf1d 100644 --- a/SessionUtilitiesKit/Database/GRDBStorage.swift +++ b/SessionUtilitiesKit/Database/GRDBStorage.swift @@ -200,20 +200,11 @@ public final class GRDBStorage { return keySpec } catch { - print("RAWR \(error.localizedDescription), \((error as? KeychainStorageError)?.code), \(errSecItemNotFound)") - switch (error, (error as? KeychainStorageError)?.code) { - // TODO: Are there other errors we know about that indicate an invalid keychain? -// errSecNotAvailable: OSStatus { get } /* No keychain is available. You may need to restart your computer. */ -// public var errSecNoSuchKeychain - - //errSecInteractionNotAllowed - case (StorageError.invalidKeySpec, _): // For these cases it means either the keySpec or the keychain has become corrupt so in order to // get back to a "known good state" and behave like a new install we need to reset the storage // and regenerate the key - // TODO: Check what this 'isRunningTests' does (use the approach to check if XCTTestCase exists instead?) if !CurrentAppContext().isRunningTests { // Try to reset app by deleting database. resetAllStorage() diff --git a/SessionUtilitiesKit/JobRunner/JobRunner.swift b/SessionUtilitiesKit/JobRunner/JobRunner.swift index 4b29aa631..4f15da4d6 100644 --- a/SessionUtilitiesKit/JobRunner/JobRunner.swift +++ b/SessionUtilitiesKit/JobRunner/JobRunner.swift @@ -846,9 +846,10 @@ private final class JobQueue { GRDBStorage.shared.write { db in guard - !permanentFailure && - maxFailureCount >= 0 && - job.failureCount + 1 < maxFailureCount + !permanentFailure && ( + maxFailureCount < 0 || + job.failureCount + 1 < maxFailureCount + ) else { SNLog("[JobRunner] \(queueContext) \(job.variant) failed permanently\(maxFailureCount >= 0 ? "; too many retries" : "")") diff --git a/SignalUtilitiesKit/Shared Views/GalleryRailView.swift b/SignalUtilitiesKit/Shared Views/GalleryRailView.swift index a4f585135..ef256f75e 100644 --- a/SignalUtilitiesKit/Shared Views/GalleryRailView.swift +++ b/SignalUtilitiesKit/Shared Views/GalleryRailView.swift @@ -205,7 +205,7 @@ public class GalleryRailView: UIView, GalleryRailCellViewDelegate { completion: { [weak self] _ in self?.stackView.arrangedSubviews.forEach { $0.removeFromSuperview() } self?.stackView.frame = oldFrame - self?.stackClippingView.isHidden = true + self?.isHidden = true self?.cellViews = [] } ) @@ -249,11 +249,11 @@ public class GalleryRailView: UIView, GalleryRailCellViewDelegate { self?.updateFocusedItem(focusedItem) self?.stackView.layoutIfNeeded() - self?.stackClippingView.isHidden = false + self?.isHidden = false updatedOldFrame = (self?.stackView.frame) .defaulting(to: oldFrame) - self?.stackView.frame = oldFrame.offsetBy( + self?.stackView.frame = updatedOldFrame.offsetBy( dx: 0, dy: oldFrame.height ) @@ -324,6 +324,7 @@ public class GalleryRailView: UIView, GalleryRailCellViewDelegate { selectedCellView?.setIsSelected(true) self.layoutIfNeeded() + self.stackView.layoutIfNeeded() switch scrollFocusMode { case .keepCentered: