refactor: process conversation MLS message add event - WPB-10172 #2120
Conversation
echoes-hq
bot
added
the
echoes: technical-roadmap/technical-debt
| senderDomain: senderDomain, | ||
| date: date | ||
| ) | ||
|
|
There was a problem hiding this comment.
In legacy code, we were handling MLS message decryption and decrypted message processing in two different places :
EventDecoder+MLS would first decode the MLS message add event payload and decrypt the MLS message.
Then, new events with the decrypted message would be manually created and propagated so the ClientMessageRequestStrategy would consume them.
| managedObjectContext: context | ||
| ) | ||
|
|
||
| } else if messageClass is ZMAssetClientMessage.Type { |
There was a problem hiding this comment.
This method is quite big because the same code is used to handle multiple events, typically here I assume this line is meant to handle the "conversation.otr-asset-add" . I kept it although we don't seem to handle that event currently.
Note: We'll reuse many methods (including this one) to handle a proteus message.
| /// Adds a message to a given conversation. | ||
| /// - parameter messageType: The type of message to add (MLS or Proteus) | ||
| // TODO: [WPB-11839] move to MessageRepository | ||
| func addMessage( |
There was a problem hiding this comment.
this will all be moved to MessageRepository when the related PR is merged.
| @@ -22,7 +22,7 @@ import Foundation | |||
| @objc public enum MessageConfirmationType: Int16 { | |||
| case delivered, read | |||
|
|
|||
| static func convert(_ zmConfirmationType: Confirmation.TypeEnum) -> MessageConfirmationType { | |||
| public static func convert(_ zmConfirmationType: Confirmation.TypeEnum) -> MessageConfirmationType { | |||
There was a problem hiding this comment.
Many legacy methods were made public to facilitate the work in this PR (not ideal but could be adjusted later on). Conversely, a lot of existing logic was factored out when we had no other option (e.g some legacy methods would take a ZMUpdatedEvent as parameter).
| genericMessage: GenericMessage, | ||
| senderID: UUID, | ||
| date: Date | ||
| ) -> Bool { |
There was a problem hiding this comment.
any reason for not having this method throw instead of relying on Bool?
| private func getGenericMessage( | ||
| from base64Message: String, | ||
| senderID: UUID, | ||
| senderDomain: String, | ||
| date: Date?, | ||
| conversation: ZMConversation, | ||
| logAttributes: LogAttributes | ||
| ) async -> (GenericMessage, GenericMessage.OneOf_Content)? { | ||
| guard let genericMessage = GenericMessage(withBase64String: base64Message), | ||
| let content = genericMessage.content else { | ||
|
|
||
| if let sender = try? await userLocalStore.fetchUser( | ||
| with: senderID, | ||
| domain: senderDomain | ||
| ) { | ||
| let systemMessage = SystemMessage( | ||
| type: .invalid, | ||
| sender: sender, | ||
| timestamp: date ?? .now | ||
| ) | ||
|
|
||
| await addSystemMessage( | ||
| systemMessage, | ||
| to: conversation | ||
| ) | ||
| } | ||
|
|
||
| WireLogger.eventProcessing.warn( | ||
| "Can't read protobuf, abort processing", | ||
| attributes: logAttributes | ||
| ) | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| return (genericMessage, content) | ||
| } |
There was a problem hiding this comment.
this method is doing two things extracting the message and adding a systemmessage, it would be great to separate the two
Test Results2 196 tests 2 196 ✅ 6m 10s ⏱️ Results for commit 0bc3353. |
|
closed, a single PR was opened for both MLS/Proteus add message events. |
WPB-10172
Key points
This PR is part of the quick sync refactoring plan and is related to processing the multiple events we receive from the backend or the push channel.
Specifically, this PR is about porting the existing implementation of the
ConversationMLSMessageAddevent.Testing
UTs cover the following use cases, ensuring that
Checklist
[WPB-XXX].UI accessibility checklist
If your PR includes UI changes, please utilize this checklist: