Skip to content

Commit

Permalink
Merge pull request #1166 from Infomaniak/fixCrashSwitch
Browse files Browse the repository at this point in the history 
fix: Realm crash when quickly deleting attachments
  • Loading branch information
@PhilippeWeidmann
PhilippeWeidmann authored Dec 7, 2023
2 parents 8f4689e + 91c50aa commit 6c84ec0
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 48 deletions.
11 changes: 6 additions & 5 deletions Mail/Components/AttachmentView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,26 @@ import MailResources
import SwiftUI

struct AttachmentView<Content: View>: View {
let attachment: Attachment
let subtitle: String
private let detachedAttachment: Attachment
private let subtitle: String

@ViewBuilder let accessory: () -> Content?

init(attachment: Attachment, subtitle: String, accessory: @escaping () -> Content? = { EmptyView() }) {
self.attachment = attachment
detachedAttachment = attachment.detached()
self.subtitle = subtitle
self.accessory = accessory
}

var body: some View {
VStack(spacing: 0) {
HStack {
IKIcon(attachment.icon, size: .large)
IKIcon(detachedAttachment.icon, size: .large)
.foregroundStyle(MailResourcesAsset.textSecondaryColor)

HStack(spacing: UIPadding.small) {
VStack(alignment: .leading, spacing: 0) {
Text(attachment.name)
Text(detachedAttachment.name)
.textStyle(.bodySmall)
.lineLimit(1)
.truncationMode(.middle)
Expand Down
2 changes: 1 addition & 1 deletion Mail/Views/AI Writer/AIModel.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ final class AIModel: ObservableObject {
init(mailboxManager: MailboxManager, draftContentManager: DraftContentManager, editedDraft: EditedDraft) {
self.mailboxManager = mailboxManager
self.draftContentManager = draftContentManager
draft = editedDraft.draft.freezeIfNeeded()
draft = editedDraft.detachedDraft
messageReply = editedDraft.messageReply
}
}
Expand Down
16 changes: 11 additions & 5 deletions Mail/Views/New Message/Attachments/AttachmentUploadCell.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,28 @@ import MailResources
import SwiftUI

struct AttachmentUploadCell: View {
private let detachedAttachment: Attachment
private let attachmentRemoved: ((Attachment) -> Void)?

@ObservedObject var uploadTask: AttachmentUploadTask

let attachment: Attachment
let attachmentRemoved: ((Attachment) -> Void)?
init(uploadTask: AttachmentUploadTask, attachment: Attachment, attachmentRemoved: ((Attachment) -> Void)?) {
self.uploadTask = uploadTask
detachedAttachment = attachment.detached()
self.attachmentRemoved = attachmentRemoved
}

var body: some View {
AttachmentView(
attachment: attachment,
subtitle: uploadTask.error != nil ? uploadTask.error!.localizedDescription : attachment.size
attachment: detachedAttachment,
subtitle: uploadTask.error != nil ? uploadTask.error!.localizedDescription : detachedAttachment.size
.formatted(.defaultByteCount)
) {
Button {
if let attachmentRemoved {
@InjectService var matomo: MatomoUtils
matomo.track(eventWithCategory: .attachmentActions, name: "delete")
attachmentRemoved(attachment)
attachmentRemoved(detachedAttachment)
}
} label: {
IKIcon(MailResourcesAsset.close, size: .small)
Expand Down
6 changes: 3 additions & 3 deletions Mail/Views/New Message/Attachments/AttachmentsHeaderView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ struct AttachmentsHeaderView: View {

var body: some View {
ZStack {
if !attachmentsManager.attachments.isEmpty {
if !attachmentsManager.liveAttachments.isEmpty {
ScrollView(.horizontal, showsIndicators: false) {
HStack(spacing: UIPadding.small) {
ForEach(attachmentsManager.attachments) { attachment in
ForEach(attachmentsManager.liveAttachments) { attachment in
AttachmentUploadCell(
uploadTask: attachmentsManager.attachmentUploadTaskOrFinishedTask(for: attachment.uuid),
attachment: attachment
) { attachmentRemoved in
attachmentsManager.removeAttachment(attachmentRemoved)
attachmentsManager.removeAttachment(attachmentRemoved.uuid)
}
}
}
Expand Down
69 changes: 44 additions & 25 deletions Mail/Views/New Message/Attachments/AttachmentsManager.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import MailCore
import PhotosUI
import SwiftUI

final class AttachmentUploadTask: ObservableObject {
@MainActor final class AttachmentUploadTask: ObservableObject {
@Published var progress: Double = 0
var task: Task<String?, Never>?
@Published var error: MailError?
Expand All @@ -33,9 +33,26 @@ final class AttachmentUploadTask: ObservableObject {
}
}

@MainActor
final class AttachmentsManager: ObservableObject {
private let draft: Draft
/// Abstract the fact some object was updated
protocol ContentUpdatable: AnyObject {
/// Call to notify the content has changed.
@MainActor func contentWillChange()
}

/// Something to track `Attachments` linked to a live `Draft`
@MainActor final class AttachmentsManager: ObservableObject, ContentUpdatable {
private let draftLocalUUID: String

/// Live `Draft` getter
private var liveDraft: Draft? {
guard let liveDraft = mailboxManager.getRealm().object(ofType: Draft.self, forPrimaryKey: draftLocalUUID),
!liveDraft.isInvalidated else {
return nil
}

return liveDraft
}

private let mailboxManager: MailboxManager
private let parallelTaskMapper = ParallelTaskMapper()
private let backgroundRealm: BackgroundRealm
Expand All @@ -44,8 +61,11 @@ final class AttachmentsManager: ObservableObject {
private let contentWillChangeSubject = PassthroughSubject<Void, Never>()
private var contentWillChangeObserver: AnyCancellable?

var attachments: [Attachment] {
return draft.attachments.filter { $0.contentId == nil }.toArray()
var liveAttachments: [Attachment] {
guard let liveDraft else {
return []
}
return liveDraft.attachments.filter { $0.contentId == nil && !$0.isInvalidated }.toArray()
}

private var attachmentUploadTasks = SendableDictionary<String, AttachmentUploadTask>()
Expand All @@ -62,8 +82,8 @@ final class AttachmentsManager: ObservableObject {
return formatter
}()

init(draft: Draft, mailboxManager: MailboxManager) {
self.draft = draft
init(draftLocalUUID: String, mailboxManager: MailboxManager) {
self.draftLocalUUID = draftLocalUUID
self.mailboxManager = mailboxManager

// Debouncing objectWillChange helps a lot scaling with numerous attachments
Expand All @@ -75,22 +95,25 @@ final class AttachmentsManager: ObservableObject {
}
}

func contentWillChange() {
contentWillChangeSubject.send()
}

func completeUploadedAttachments() {
for attachment in attachments {
for attachment in liveAttachments {
let uploadTask = attachmentUploadTaskOrCreate(for: attachment.uuid)
uploadTask.progress = 1
}
contentWillChangeSubject.send()
contentWillChange()
}

private func updateAttachment(oldAttachment: Attachment, newAttachment: Attachment) async {
guard let oldAttachment = draft.attachments.first(where: { $0.uuid == oldAttachment.uuid }) else {
guard let oldAttachment = liveDraft?.attachments.first(where: { $0.uuid == oldAttachment.uuid }) else {
return
}

let oldAttachmentUUID = oldAttachment.uuid
let newAttachmentUUID = newAttachment.uuid
let primaryKey = draft.localUUID

if oldAttachmentUUID != newAttachmentUUID {
attachmentUploadTasks[newAttachmentUUID] = attachmentUploadTasks[oldAttachmentUUID]
Expand All @@ -99,7 +122,7 @@ final class AttachmentsManager: ObservableObject {

await backgroundRealm.execute { realm in
try? realm.write {
guard let draftInContext = realm.object(ofType: Draft.self, forPrimaryKey: primaryKey) else {
guard let draftInContext = realm.object(ofType: Draft.self, forPrimaryKey: self.draftLocalUUID) else {
return
}

Expand All @@ -112,7 +135,7 @@ final class AttachmentsManager: ObservableObject {
}
}

contentWillChangeSubject.send()
contentWillChange()
}

/// Lookup and return. New object created and returned instead
Expand Down Expand Up @@ -147,14 +170,11 @@ final class AttachmentsManager: ObservableObject {
return attachment
}

func removeAttachment(_ attachment: Attachment) {
let attachmentUUID = attachment.uuid
let primaryKey = draft.localUUID

func removeAttachment(_ attachmentUUID: String) {
Task {
await backgroundRealm.execute { realm in
try? realm.write {
guard let draftInContext = realm.object(ofType: Draft.self, forPrimaryKey: primaryKey) else {
guard let draftInContext = realm.object(ofType: Draft.self, forPrimaryKey: self.draftLocalUUID) else {
return
}

Expand All @@ -169,18 +189,17 @@ final class AttachmentsManager: ObservableObject {
attachmentUploadTasks[attachmentUUID]?.task?.cancel()
attachmentUploadTasks.removeValue(forKey: attachmentUUID)

contentWillChangeSubject.send()
contentWillChange()
}
}

private func addLocalAttachment(attachment: Attachment) async -> Attachment? {
attachmentUploadTasks[attachment.uuid] = AttachmentUploadTask()
let primaryKey = draft.localUUID

var detached: Attachment?
await backgroundRealm.execute { realm in
try? realm.write {
guard let draftInContext = realm.object(ofType: Draft.self, forPrimaryKey: primaryKey) else {
guard let draftInContext = realm.object(ofType: Draft.self, forPrimaryKey: self.draftLocalUUID) else {
return
}

Expand All @@ -190,7 +209,7 @@ final class AttachmentsManager: ObservableObject {
detached = attachment.detached()
}

contentWillChangeSubject.send()
contentWillChange()
return detached
}

Expand Down Expand Up @@ -264,10 +283,10 @@ final class AttachmentsManager: ObservableObject {
do {
let url = try await attachment.writeToTemporaryURL()
let updatedAttachment = await updateLocalAttachment(url: url, attachment: localAttachment)
let totalSize = attachments.map(\.size).reduce(0) { $0 + $1 }
let totalSize = liveAttachments.map(\.size).reduce(0) { $0 + $1 }
guard totalSize < Constants.maxAttachmentsSize else {
globalError = MailError.attachmentsSizeLimitReached
removeAttachment(updatedAttachment)
removeAttachment(updatedAttachment.uuid)
return nil
}

Expand Down
5 changes: 3 additions & 2 deletions Mail/Views/New Message/ComposeMessageBodyView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,14 @@ struct ComposeMessageBodyView: View {

struct ComposeMessageBodyView_Previews: PreviewProvider {
static var previews: some View {
ComposeMessageBodyView(draft: Draft(),
let draft = Draft()
ComposeMessageBodyView(draft: draft,
editorModel: .constant(RichTextEditorModel()),
editorFocus: .constant(false),
currentSignature: .constant(nil),
isShowingAIPrompt: .constant(false),
attachmentsManager: AttachmentsManager(
draft: Draft(),
draftLocalUUID: draft.localUUID,
mailboxManager: PreviewHelper.sampleMailboxManager
),
alert: NewMessageAlert(),
Expand Down
8 changes: 4 additions & 4 deletions Mail/Views/New Message/ComposeMessageView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,18 @@ struct ComposeMessageView: View {
init(editedDraft: EditedDraft, mailboxManager: MailboxManager, attachments: [Attachable] = []) {
messageReply = editedDraft.messageReply

Self.writeDraftToRealm(mailboxManager.getRealm(), draft: editedDraft.draft)
_draft = StateRealmObject(wrappedValue: editedDraft.draft)
Self.writeDraftToRealm(mailboxManager.getRealm(), draft: editedDraft.detachedDraft)
_draft = StateRealmObject(wrappedValue: editedDraft.detachedDraft)

let currentDraftContentManager = DraftContentManager(
incompleteDraft: editedDraft.draft,
incompleteDraft: editedDraft.detachedDraft,
messageReply: editedDraft.messageReply,
mailboxManager: mailboxManager
)
draftContentManager = currentDraftContentManager

self.mailboxManager = mailboxManager
_attachmentsManager = StateObject(wrappedValue: AttachmentsManager(draft: editedDraft.draft,
_attachmentsManager = StateObject(wrappedValue: AttachmentsManager(draftLocalUUID: editedDraft.detachedDraft.localUUID,
mailboxManager: mailboxManager))
_initialAttachments = State(wrappedValue: attachments)

Expand Down
2 changes: 1 addition & 1 deletion Mail/Views/Thread/MessageHeaderView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ struct MessageHeaderView: View {
eventWithCategory: .newMessage,
action: .data,
name: "openLocalDraft",
value: !(mainViewState.editedDraft?.draft.isLoadedRemotely ?? false)
value: !(mainViewState.editedDraft?.detachedDraft.isLoadedRemotely ?? false)
)
} else if message.originalThread?.messages.isEmpty == false {
withAnimation {
Expand Down
11 changes: 9 additions & 2 deletions MailCore/Models/EditedDraft.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,21 @@

import Foundation

/// Something to track an edited `Draft`
public struct EditedDraft: Identifiable {
public var id: ObjectIdentifier {
return draft.id
return detachedDraft.id
}

public let draft: Draft
public let detachedDraft: Draft

public let messageReply: MessageReply?

init(draft: Draft, messageReply: MessageReply?) {
detachedDraft = draft.detached()
self.messageReply = messageReply
}

public static func new() -> EditedDraft {
return EditedDraft(draft: Draft(localUUID: UUID().uuidString), messageReply: nil)
}
Expand Down

0 comments on commit 6c84ec0

Please sign in to comment.