feat: Multipart message support (#WPB-15746) #3320

sbakhtiarov · 2025-03-05T12:53:24Z

WPB-15746 [Android] Send message with cell attachments

https://wearezeta.atlassian.net/browse/WPB-15746

What's new in this PR?

Support for sending multipart messages for conversations with Wire Cell support. New message type is only supported for sending assets uploaded to wire cell storage. Support for sending regular encrypted assets with new Multipart message will be added later (so there are some TODOs in the code for handling regular assets with Multipart)

Some interactions with the new message type are no supported at the moment:

Replies/quotes - not supported
Edit - only for editing text (no attachments edit)
Delete - work in progress to remove assets when deleting a message.

1. Protocol update
New multipart message combining text with multiple attachments (Regular or Cell). Update code for packing / unpacking and all handlers.

2. Database
99.sqm migration file is a summary of new/updated tables.
[Updated] Conversation/ConversationDetails - added wire_cell column to enable/disable wire cell per conversation. Currently just a local client-side value. Once BE is ready it will be synced with the server.
[Updated]MessageAttachmentDraft: Asset metadata is now stored in the database for attachment drafts for sending in Multipart message.
[New]MessageAttachments: new table for storing message attachments with one-to-many relation (one message/conversation to many asset ids)
[Updated] MessageDetailsView : updated to return list of attachments for each message from the MessageAttachments table. Implementation is similar to handling reactions/mentions for messages.

3. Use case
[New]SendMultipartMessageUseCase new use case for sending multipart message. For conversations with Wire Cell support the new Multipart message is used, for regular conversations assets are sent as individual asset messages.
[Updated] RetryFailedMessageUseCase updated to support re-sending of the failed Multipart message.

github-actions · 2025-03-05T13:01:42Z

Test Results

318 tests 309 ✅ 41s ⏱️
6 suites 9 💤
6 files 0 ❌

Results for commit 8688912.

♻️ This comment has been updated with latest results.

datadog-wireapp · 2025-03-05T13:26:48Z

Datadog Report

Branch report: multipart-message
Commit report: 3217c7d
Test service: kalium-jvm

✅ 0 Failed, 347 Passed, 12 Skipped, 15.61s Total Time

yamilmedina

Looking good 🚀 I have some small solvable comments about redundant modifiers, docs. As well as something maybe more demanding to solve regarding entities accessed in the use case layer.

cells/src/commonMain/kotlin/com/wire/kalium/cells/CellsScope.kt

cells/src/commonMain/kotlin/com/wire/kalium/cells/domain/MessageAttachmentDraftRepository.kt

cells/src/commonMain/kotlin/com/wire/kalium/cells/domain/usecase/AddAttachmentDraftUseCase.kt

yamilmedina · 2025-03-06T12:57:04Z

cells/src/commonMain/kotlin/com/wire/kalium/cells/CellsScope.kt


    public val observeFiles: ObserveCellFilesUseCase
        get() = ObserveCellFilesUseCaseImpl(conversationsDAO, cellsRepository)
+
+    public val enableWireCell: SetWireCellForConversationUseCase
+        get() = SetWireCellForConversationUseCase(conversationsDAO)


I think this (if not mistaken) should fail by our arch tests, since the idea of use cases is to use repositories to fulfill their needs, and avoiding accessing directly the entities.

It would be ideal to not bypass this.

EDIT: I think the tests are not running since this is a new module, but also exposed to consumers. We should add the tests to this project as well or maybe we can solve it later to have it resolved by CI at least and/or gradle itself.

I will fix it. I think I just made a shortcut here by using Dao directly, but it is better to do it via repository.

yamilmedina · 2025-03-06T12:58:36Z

cells/src/commonMain/kotlin/com/wire/kalium/cells/domain/usecase/DownloadCellFileUseCase.kt

+import kotlinx.coroutines.withContext
+import okio.Path
+
+public class DownloadCellFileUseCase internal constructor(


Suggestion: We try to keep our Usecases documented with Kdocs, for the never-ending desire of using Kalium for other clients.

Yes, will add documentation for this one.

yamilmedina · 2025-03-06T13:14:24Z

cells/src/commonMain/kotlin/com/wire/kalium/cells/domain/usecase/AddAttachmentDraftUseCase.kt


-        val destNodePath = "$ROOT_CELL/$conversationId/$fileName"
+        val cellName = conversationDao.getCellName(QualifiedIDEntity(conversationId.value, conversationId.domain))


As a prev. comment, we shouldn't be able to use. Entities on the use cases since they can leak by mistake to consumers.

Will replace with repository method call.

vitorhugods

LGTM. Just some small details / nits and general questions that reflect my lack of context :P

vitorhugods · 2025-03-06T12:59:18Z

cells/src/commonMain/kotlin/com/wire/kalium/cells/domain/model/NodePreview.kt

+    val density: Int,
+)


As I have zero context... what is density? :P Could you maybe add some Kdocs? Specially around the domain / usecase packages.

This is a preview thumbnail resolution (max dimension). Server generates multiple 300, 1024 but I am using only the best one with 1024

vitorhugods · 2025-03-06T13:02:59Z

cells/src/commonMain/kotlin/com/wire/kalium/cells/domain/usecase/DownloadCellFileUseCase.kt

+public class DownloadCellFileUseCase internal constructor(
+    private val cellsRepository: CellsRepository,


We have some tests on logic that check for things like "data layer can't depend on use cases" and "all use cases must be documented", etc.

I just realised that with modularisation, this can be left without check. I don't think it's a rule that is always necessary. But it's better to be 10% annoying than 90% missing in documentation. This usecase seems pretty self-explanatory.

In general, with this comment, I just want to raise awareness about the tests we had on logic and that are no longer being checked in feature modules.

Something that we - as a team - should resolve at some point.

vitorhugods · 2025-03-06T13:06:50Z

cells/src/commonMain/kotlin/com/wire/kalium/cells/domain/usecase/GetPreviewUrlUseCase.kt

+                delay(500)
+            }
+
+        } while (previewList.isEmpty() && retry++ < 20)


Not a deal-breaker. But I think it's worth going the small effort of extracting this 20 to a const val MAX_RETRY_COUNT = 20. Makes it easier to quickly find and change these values if/when needed, also helps a bit with readability in the future.

This Use Case is work in progress. I should have mentioned it in the comment. We are still working on the final solution for handling asset preview thumbnails.

vitorhugods · 2025-03-06T13:07:49Z

cells/src/commonMain/kotlin/com/wire/kalium/cells/domain/usecase/GetPreviewUrlUseCase.kt

+            cellsRepository.savePreviewUrl(assetId, it.url)
+        }
+
+        return Unit.right()


Do we ever return Either.Left inthis UseCase?

Will refactor it.

vitorhugods · 2025-03-06T13:25:33Z

...c/src/commonMain/kotlin/com/wire/kalium/logic/feature/message/SendMultipartMessageUseCase.kt

+            content = if (isCellEnabled) {
+                MessageContent.Multipart(


Could also check if there are attachments?

This use case should only be called to send a message with attachments, without attachments client must send a simple Text message. Maybe it could even throw error if it is used to send message without attachments.

vitorhugods · 2025-03-06T13:27:32Z

...c/src/commonMain/kotlin/com/wire/kalium/logic/feature/message/SendMultipartMessageUseCase.kt

+            attachments.forEach { attachment ->
+                with(attachment as CellAssetContent) {


I wonder if it is possible to add attachments, then the team enables cells, then we have a crash here of some sort, as this cast could fail.

The implementation for sending regular encrypted attachments is not done yet. I will implement it later and handle the other content type.

sonarqubecloud · 2025-03-07T11:41:40Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

github-actions bot assigned sbakhtiarov Mar 5, 2025

github-actions bot added 🚨 Potential breaking changes 👕 size: XXL Fix PR Title 🤦‍♂️ labels Mar 5, 2025

sbakhtiarov changed the title ~~Multipart message~~ feat: Multipart message support (#WPB-15746) Mar 5, 2025

sbakhtiarov removed the Fix PR Title 🤦‍♂️ label Mar 5, 2025

github-actions bot added the type: feature ✨ label Mar 5, 2025

sbakhtiarov requested review from Garzas, vitorhugods, saleniuk and MohamadJaara and removed request for vitorhugods March 5, 2025 12:56

sbakhtiarov added 4 commits March 5, 2025 15:26

feat: add wire cell support flag for group conversation (#WPB-15741)

2648157

feat: new table for multipart attachments (#WPB-15741)

463d6f2

feat: basic support for multipart message (#WPB-15741)

04d4c2d

feat: support for download progress (#WPB-15741)

e690d03

sbakhtiarov force-pushed the multipart-message branch from 4a91b9b to e690d03 Compare March 5, 2025 14:27

fixing unit tests (#WPB-15741)

1ad194c

sbakhtiarov force-pushed the multipart-message branch from e926538 to 1ad194c Compare March 5, 2025 16:04

yamilmedina reviewed Mar 6, 2025

View reviewed changes

vitorhugods reviewed Mar 6, 2025

View reviewed changes

code review comments (#WPB-15741)

bccc133

sbakhtiarov requested review from yamilmedina and vitorhugods March 7, 2025 08:42

sbakhtiarov added 3 commits March 7, 2025 11:12

fixing unit tests (#WPB-15746)

e01bd9b

fixing unit tests (#WPB-15746)

d4d6d91

fixing unit tests (#WPB-15746)

361681f

fixing unit tests (#WPB-15746)

8688912

yamilmedina approved these changes Mar 7, 2025

View reviewed changes

sbakhtiarov merged commit 48755b7 into epic/wire-cells Mar 10, 2025
17 of 20 checks passed

sbakhtiarov deleted the multipart-message branch March 10, 2025 17:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Multipart message support (#WPB-15746) #3320

feat: Multipart message support (#WPB-15746) #3320

sbakhtiarov commented Mar 5, 2025 •

edited by github-actions bot

Loading

github-actions bot commented Mar 5, 2025 •

edited

Loading

datadog-wireapp bot commented Mar 5, 2025 •

edited

Loading

yamilmedina left a comment

yamilmedina Mar 6, 2025

sbakhtiarov Mar 6, 2025

yamilmedina Mar 6, 2025

sbakhtiarov Mar 6, 2025

yamilmedina Mar 6, 2025

sbakhtiarov Mar 6, 2025

vitorhugods left a comment

vitorhugods Mar 6, 2025

sbakhtiarov Mar 6, 2025

vitorhugods Mar 6, 2025

vitorhugods Mar 6, 2025

sbakhtiarov Mar 6, 2025

vitorhugods Mar 6, 2025

sbakhtiarov Mar 6, 2025

vitorhugods Mar 6, 2025

sbakhtiarov Mar 6, 2025

vitorhugods Mar 6, 2025

sbakhtiarov Mar 6, 2025

sonarqubecloud bot commented Mar 7, 2025


		val destNodePath = "$ROOT_CELL/$conversationId/$fileName"
		val cellName = conversationDao.getCellName(QualifiedIDEntity(conversationId.value, conversationId.domain))

		public class DownloadCellFileUseCase internal constructor(
		private val cellsRepository: CellsRepository,

		attachments.forEach { attachment ->
		with(attachment as CellAssetContent) {

feat: Multipart message support (#WPB-15746) #3320

feat: Multipart message support (#WPB-15746) #3320

Conversation

sbakhtiarov commented Mar 5, 2025 • edited by github-actions bot Loading

What's new in this PR?

github-actions bot commented Mar 5, 2025 • edited Loading

Test Results

datadog-wireapp bot commented Mar 5, 2025 • edited Loading

Datadog Report

yamilmedina left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitorhugods left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Mar 7, 2025

Quality Gate passed

sbakhtiarov commented Mar 5, 2025 •

edited by github-actions bot

Loading

github-actions bot commented Mar 5, 2025 •

edited

Loading

datadog-wireapp bot commented Mar 5, 2025 •

edited

Loading