Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: process conversation proteus/MLS message add event - WPB-10174 #2164

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

jullianm
Copy link
Contributor

@jullianm jullianm commented Nov 18, 2024

WPB-10174

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 ConversationMLSMessageAdd and the ConversationProteusMessageAdd event.

A previous PR was opened and is now closed since this PR contains both implementation (Proteus and MLS) - related tickets:

Testing

  • MLS and Proteus messages are correctly decrypted (using dedicated decryptors)
  • MessageRepository addMessage for MLS/Proteus methods are correctly invoked.
  • A Proteus message is added to a conversation (MessageLocalStoreTests)
  • A Proteus with a large payload message (externalData) is added to a conversation (MessageLocalStoreTests)
  • A MLS message is added to a conversation (MessageLocalStoreTests)

Checklist

  • Title contains a reference JIRA issue number like [WPB-XXX].
  • Description is filled and free of optional paragraphs.
  • Adds/updates automated tests.

@echoes-hq echoes-hq bot added the echoes: technical-roadmap/technical-debt Changes intended at mitigating risks label Nov 18, 2024
/// The decrypted current message + decrypted buffered messages
/// along with the related sender client ID for each message.

public var decryptedMessages: [DecryptedMessage] = []
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar to the Proteus add event with the message content property, this property will be filled in the decryptor when the message + buffered messages are decrypted

)

var decryptedEvent = eventData
decryptedEvent.decryptedMessages = decryptedMessages
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filled here

)

do {
let decryptedEventData = try await mlsMessageDecryptor.decryptedEventData(from: eventData)
Copy link
Contributor Author

@jullianm jullianm Nov 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar to what we did for the proteusMessageAdd, we use the decryptor to decrypt the MLS messages from the event and return the same event filled with the decrypted messages (current one and buffered ones)

}

// swiftlint:disable:next todo_requires_jira_link
// TODO: Move to UserClientsLocalStore when related PR is merged
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the related PR

)
}

func testAddProteusMessage_It_Adds_Message_To_Conversation() async throws {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testing a regular message is added to conversation

)
}

func testAddProteusMessage_It_Adds_Big_Payload_Message_To_Conversation() async throws {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testing a large message payload (external) is added to the conversation

@@ -46,7 +46,7 @@ extension ZMClientMessage: CompositeMessageData {

// MARK: - ButtonStates Interface
extension ZMClientMessage {
static func updateButtonStates(withConfirmation confirmation: ButtonActionConfirmation,
public static func updateButtonStates(withConfirmation confirmation: ButtonActionConfirmation,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some methods were made public to facilitate the work in this PR, many others had to be factored out because they could not be reused (the ones that take a ZMUpdateEvent as parameter)

Comment on lines +53 to +55
WireLogger.mls.error(
"failed to add mls message: conversation not found in db"
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we throw I guess we can remove logging the error

return results

} catch {
WireLogger.mls.warn(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
WireLogger.mls.warn(
WireLogger.mls.error(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
echoes: technical-roadmap/technical-debt Changes intended at mitigating risks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants