feat: keep messages pending while offline - WPB-25320

WireData/Sources/WireData/Schema/Databases Changelog.md

@@ -14,6 +14,10 @@ As it is hard to spot changes from version to version of database models (.xcdat
 
 * remove `Conversation.commitPendingProposalDate`, superseded by the `PendingProposalTimer` entity (values are moved by `PendingProposalTimerMigrationAction` when migrating to 2.143.0). Also remove the `@NSManaged` property in `ZMConversation` and its remaining usages.
 
+### 2.146.0
+
+* added `ClientMessage.sendAttemptStarted`, set before a pending message is first sent, so it is only edited in place while it cannot have reached the backend
+
 ### 2.145.0
 
 * added optional `Meeting.invitedEmails` to preserve email invitees in the Meetings list.

WireData/Sources/WireData/Schema/zmessaging.xcdatamodeld/.xccurrentversion

@@ -3,6 +3,6 @@
 <plist version="1.0">
 <dict>
 	<key>_XCCurrentVersionName</key>
-	<string>zmessaging2.145.0.xcdatamodel</string>
+	<string>zmessaging2.146.0.xcdatamodel</string>
 </dict>
 </plist>

wire-ios-data-model/Source/ManagedObjectContext/Migration/CoreDataMessagingMigrationVersion.swift

@@ -30,6 +30,7 @@ enum CoreDataMessagingMigrationVersion: String, CoreDataMigrationVersion {
     // MARK: -
 
     // Note: add new versions here in first position!
+    case v146 = "zmessaging2.146.0"
     case v145 = "zmessaging2.145.0"
     case v144 = "zmessaging2.144.0"
     case v143 = "zmessaging2.143.0"
@@ -99,8 +100,10 @@ enum CoreDataMessagingMigrationVersion: String, CoreDataMigrationVersion {
 
     var nextVersion: Self? {
         switch self {
-        case .v145:
+        case .v146:
             nil
+        case .v145:
+            .v146
         case .v144:
             .v145
         case .v143:

wire-ios-data-model/Source/Model/Conversation/ZMConversation+Message.swift

@@ -430,6 +430,8 @@ public extension ZMConversation {
         }
 
         let message = ZMClientMessage(nonce: nonce, managedObjectContext: moc)
+        // Only newly composed outgoing messages are known to have no previous send attempt.
+        message.sendAttemptStarted = false
         configure?(message)
 
         do {

wire-ios-data-model/Source/Model/Message/ConversationMessage.swift

@@ -379,7 +379,7 @@ public extension ZMMessage {
     }
 
     @objc var canBeDeleted: Bool {
-        deliveryState != .pending
+        deliveryState != .pending || (self as? ZMClientMessage)?.isDeletableWhilePending == true
     }
 
     @objc var hasBeenDeleted: Bool {

wire-ios-data-model/Source/Model/Message/ZMClientMessage+TextMessageData.swift

@@ -111,6 +111,104 @@ extension ZMClientMessage: TextMessageData {
         linkPreviewState = fetchLinkPreview ? .waitingToBeProcessed : .done
         linkAttachments = nil
         delivered = false
+        sendAttemptStarted = false
+    }
+
+    // MARK: - Pending messages
+
+    /// Whether the message waits for the network instead of failing when the device is offline.
+    public var staysPendingWhileOffline: Bool {
+        DeveloperFlag.offlinePendingMessages.isOn && !isEphemeral
+    }
+
+    /// Whether the text can be replaced without sending an edit, because the message is pending and
+    /// hasn't been sent yet. Recipients then only ever see the new text, without an edited marker.
+    public var isEditableInPlace: Bool {
+        guard
+            DeveloperFlag.offlinePendingMessages.isOn,
+            !isEphemeral,
+            !isExpired,
+            !delivered,
+            !sendAttemptStarted,
+            let sender, sender.isSelfUser
+        else {
+            return false
+        }
+
+        switch underlyingMessage?.content {
+        case .text:
+            return true
+        case let .edited(messageEdit):
+            if case .text = messageEdit.content {
+                return true
+            }
+            return false
+        default:
+            return false
+        }
+    }
+
+    /// Creates the text for `replacePendingText(with:fetchLinkPreview:)`. Call it on the context of the
+    /// mentioned users.
+    public func makePendingText(_ text: String, mentions: [Mention]) -> Text {
+        Text(content: text, mentions: mentions, linkPreviews: [], replyingTo: quote as? ZMOTRMessage)
+    }
+
+    /// Replaces the text of a pending message that hasn't been sent yet, keeping its nonce.
+    ///
+    /// Call it on the sync context, which also marks the message before sending it, so the text can't
+    /// change while a send reads it.
+    ///
+    /// - Returns: `false` if the message is no longer editable in place, e.g. because sending started.
+    @discardableResult
+    public func replacePendingText(with text: Text, fetchLinkPreview: Bool) -> Bool {
+        guard isEditableInPlace, let nonce, let content = underlyingMessage?.content else { return false }
+
+        let updatedMessage: GenericMessage
+        switch content {
+        case let .edited(messageEdit):
+            guard let originalNonce = UUID(uuidString: messageEdit.replacingMessageID) else { return false }
+            updatedMessage = GenericMessage(
+                content: MessageEdit(replacingMessageID: originalNonce, text: text),
+                nonce: nonce
+            )
+        default:
+            updatedMessage = GenericMessage(content: text, nonce: nonce)
+        }
+
+        do {
+            try setUnderlyingMessage(updatedMessage)
+        } catch {
+            Logging.messageProcessing.warn("Failed to replace pending text. Reason: \(error.localizedDescription)")
+            return false
+        }
+
+        updateNormalizedText()
+        linkPreviewState = fetchLinkPreview ? .waitingToBeProcessed : .done
+        linkAttachments = nil
+        return true
+    }
+
+    /// Whether the message can be deleted without sending a delete, because it's a new pending message
+    /// that hasn't been sent yet, so nobody else has it.
+    public var isDeletableWhilePending: Bool {
+        guard isEditableInPlace, case .text = underlyingMessage?.content else { return false }
+        return true
+    }
+
+    /// Deletes a pending message that hasn't been sent yet, without sending a delete.
+    ///
+    /// Call it on the sync context, which also marks the message before sending it, so it can't be
+    /// deleted while a send reads it.
+    ///
+    /// - Returns: `false` if the message can no longer be deleted this way, e.g. because sending started.
+    @discardableResult
+    public func deletePendingMessage() -> Bool {
+        guard isDeletableWhilePending, let managedObjectContext else { return false }
+
+        removeClearingSender(true)
+        managedObjectContext.delete(self)
+        return true
     }
 
     var multipartAttachments: [GenericMessageProtocol.Attachment]? {

wire-ios-data-model/Source/Model/Message/ZMClientMessage.swift

@@ -35,6 +35,11 @@ public class ZMClientMessage: ZMOTRMessage {
     /// Link Preview state
     @NSManaged public var updatedTimestamp: Date?
 
+    /// Whether a request with the current nonce may have reached the backend. Defaults to true for
+    /// stored messages with unknown send history. New outgoing messages explicitly start with false,
+    /// and every send attempt sets it to true regardless of the offline-pending feature flag.
+    @NSManaged public var sendAttemptStarted: Bool
+
     /// In memory cache
     var cachedUnderlyingMessage: GenericMessage?
 
@@ -44,7 +49,7 @@ public class ZMClientMessage: ZMOTRMessage {
 
     open override var ignoredKeys: Set<AnyHashable>? {
         (super.ignoredKeys ?? Set())
-            .union([#keyPath(updatedTimestamp)])
+            .union([#keyPath(updatedTimestamp), #keyPath(sendAttemptStarted)])
     }
 
     public override var updatedAt: Date? {

wire-ios-data-model/Tests/ManagedObjectContext/DatabaseMigrationTests+FixDuplicateOneOnOneConversations.swift

@@ -17,7 +17,9 @@
 //
 
 import Foundation
+import GenericMessageProtocol
 import WireDataModelSupport
+
 @testable import WireDataModel
 
 final class DatabaseMigrationTests_FixDuplicateOneOnOneConversations: XCTestCase {
@@ -75,14 +77,14 @@
                     oldOneOnOneConversation.messageProtocol = .proteus
                     oldOneOnOneConversation.userDefinedName = nil
 
-                    try model.addTextMessages(
+                    try Scaffolding.addTextMessages(
                         to: oldOneOnOneConversation,
                         messagePrefix: "oldConv from me",
                         sender: selfUser,
                         count: 3,
                         in: context
                     )
-                    try model.addTextMessages(
+                    try Scaffolding.addTextMessages(
                         to: oldOneOnOneConversation,
                         messagePrefix: "oldConv from otherUser",
                         sender: otherUser,
@@ -108,14 +110,14 @@
                     // setup
                     otherUser.setValue(newOneOnOneConversation, forKey: "oneOnOneConversation")
 
-                    try model.addTextMessages(
+                    try Scaffolding.addTextMessages(
                         to: newOneOnOneConversation,
                         messagePrefix: "newConv from me",
                         sender: selfUser,
                         count: 1,
                         in: context
                     )
-                    try model.addTextMessages(
+                    try Scaffolding.addTextMessages(
                         to: newOneOnOneConversation,
                         messagePrefix: "newConv from otherUser",
                         sender: otherUser,
@@ -134,7 +136,7 @@
             postMigrationAction: { context in
 
                 try context.performAndWait {
                     let selfUser = ZMUser.selfUser(in: context)
 
                     let conversation = ZMConversation.fetch(
                         with: Scaffolding.oldOneOnOneConversationID,
@@ -181,5 +183,31 @@
         static let otherUserQualifiedID = QualifiedID(uuid: .create(), domain: Self.domain)
         static let oldOneOnOneConversationID = UUID(uuidString: "11118744-5514-45BB-A145-0B6F37856CDA")!
         static let newOneOnOneConversationID = UUID(uuidString: "87CF8744-5514-45BB-A145-0B6F37856CDA")!
+
+        /// Creates messages using only fields available in the 2.118.0 source schema.
+        /// `ModelHelper.addTextMessages` calls today's composition API, which writes `sendAttemptStarted`
+        /// (added in 2.146.0). The old model has no setter for that property and raises an Objective-C exception.
+        /// Build the fixture directly so new composition requirements don't affect setup before migration.
+        static func addTextMessages(
+            to conversation: ZMConversation,
+            messagePrefix: String,
+            sender: ZMUser,
+            count: Int,
+            in context: NSManagedObjectContext
+        ) throws {
+            for index in 0 ..< count {
+                let nonce = UUID()
+                let message = ZMClientMessage(nonce: nonce, managedObjectContext: context)
+                message.sender = sender
+                message.visibleInConversation = conversation
+
+                let messageData = ZMGenericMessageData.insertNewObject(in: context)
+                try messageData.setGenericMessage(GenericMessage(
+                    content: Text(content: "\(messagePrefix) \(index)"),
+                    nonce: nonce
+                ))
+                messageData.message = message
+            }
+        }
     }
 }

wire-ios-data-model/Tests/ManagedObjectContext/Migrations/CoreDataMessagingMigrationVersionTests.swift

@@ -33,6 +33,44 @@ final class CoreDataMessagingMigrationVersionTests: XCTestCase {
         )
     }
 
+    func testMigrationFrom145To146TreatsPreviousSendHistoryAsUnknown() throws {
+        let helper = DatabaseMigrationHelper()
+        let sourceModel = try helper.createObjectModel(version: "2.145.0")
+        let destinationModel = try helper.createObjectModel(version: "2.146.0")
+        let mapping = try NSMappingModel.inferredMappingModel(
+            forSourceModel: sourceModel,
+            destinationModel: destinationModel
+        )
+        let storeDirectory = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+        try FileManager.default.createDirectory(at: storeDirectory, withIntermediateDirectories: true)
+        defer { try? FileManager.default.removeItem(at: storeDirectory) }
+        let nonce = UUID()
+
+        try helper.migrateStore(
+            sourceVersion: "2.145.0",
+            destinationVersion: "2.146.0",
+            mappingModel: mapping,
+            storeDirectory: storeDirectory,
+            preMigrationAction: { context in
+                try context.performAndWait {
+                    let entity = try XCTUnwrap(NSEntityDescription.entity(forEntityName: "ClientMessage", in: context))
+                    let message = NSManagedObject(entity: entity, insertInto: context)
+                    message.setValue(nonce, forKey: "nonce")
+                    try context.save()
+                }
+            },
+            postMigrationAction: { context in
+                try context.performAndWait {
+                    let messages = try context.fetch(NSFetchRequest<NSManagedObject>(entityName: "ClientMessage"))
+                    XCTAssertEqual(messages.count, 1)
+                    let message = try XCTUnwrap(messages.first)
+                    XCTAssertEqual(message.value(forKey: "nonce") as? UUID, nonce)
+                    XCTAssertEqual(message.value(forKey: "sendAttemptStarted") as? Bool, true)
+                }
+            }
+        )
+    }
+
     func testMigrationFrom144To145PreservesMeetingAndDefaultsInvitedEmails() throws {
         let helper = DatabaseMigrationHelper()
         let sourceModel = try helper.createObjectModel(version: "2.144.0")