feat: port Wire Android services & use cases - WPB-16148 #2920

El-Fitz · 2025-04-18T14:31:39Z

WPB-16148 [iOS] Implement usecase for upload to WireCells

Issue

Ports and integrates the Android WireCells services & use cases interfaces & implementations to the iOS app.

Remaining

Tests for this PR will be done in a follow up ticket: https://wearezeta.atlassian.net/browse/WPB-17490

Testing

None of this code is used yet so cannot be manually tested

Checklist

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

… for drafts attachments

…e:dataPath:metadata:uploadStatus:)` compile error

Perhaps we can delete these but I will comment them for the time being

It is not clear to me what this TODO means.

…D:)` async

# Conflicts: # wire-ios-data-model/Resources/zmessaging.xcdatamodeld/zmessaging2.125.0.xcdatamodel/contents # wire-ios-data-model/WireDataModel.xcodeproj/project.pbxproj

This commit doesn't contain any new/updated entities - just the new model

github-actions · 2025-05-02T15:43:06Z

Test Results

2 788 tests 2 783 ✅ 4m 57s ⏱️
374 suites 1 💤
4 files 4 ❌

For more details on these failures, see this check.

Results for commit 11f060d.

♻️ This comment has been updated with latest results.

datadog-wireapp · 2025-05-02T15:45:10Z

Datadog Report

Branch report: feat/wire-cells-upgrade-service
Commit report: c35a2ea
Test service: wire-ios-mono

❌ 4 Failed (0 Known Flaky), 2783 Passed, 1 Skipped, 4m 36.22s Total Time

❌ Failed Tests (4)

testThatItReturnsAllKeysChangedWhenSnapshotDoesNotExist() - WireDataModelTests.SnapshotCenterTests - Details

Expand for error

 SnapshotCenterTests.swift:209: XCTAssertEqual failed: ("["internalIsArchived", "lastUnreadKnockDate", "mlsGroupID", "internalEstimatedUnreadSelfReplyCount", "syncedMessageDestructionTimeout", "commitPendingProposalDate", "silencedChangedTimestamp", "groupType", "clearedTimeStamp", "lastReadServerTimeStamp", "mlsStatus", "cellName", "hasReadReceiptsEnabled", "needsToCalculateUnreadMessages", "legalHoldStatus", "accessRoleStringsV2", "lastServerTimeStamp", "modifiedKeys", "isSelfAnActiveMember", "nonTeamRoles", "userDefinedName", "language", "messageProtocol", "draftMessageData", "accessModeStrings", "localMessageDestructionTimeout", "normalizedUserDefinedName", "lastServerSyncedActiveParticipants", "migratedToMLS", "isForcedReadOnly", "archivedChangedTimestamp", "isPendingInitialFetch", "mlsVerificationStatus", "voiceChannel", "domain", "teamRemoteIdentifier_data", "primaryKey", "mutedStatus", "lastUnreadMissedCallDate", "conversationType", "draftMessageNonce", "internalEstimatedUnreadCount", "epoch", "privateChannelPermission", "teamRemoteIdentifier", "isDeletedRemotely", "needsToBeUpdatedFromBackend", "isPendingMetadataRefresh", "remoteIdentifier", "hiddenMessages", "hasUnreadUnsentMessage", "ciphersuite", "lastModifiedDate", "participantRoles", "needsToVerifyLegalHold", "securityLevel", "accessRoleString", "needsToDownloadRoles", "internalEstimatedUnreadSelfMentionCount", "remoteIdentifier_data", "labels", "wireCellsMessageAttachmentDrafts", "allMessages"]") is not equal to ("["archivedChangedTimestamp", "normalizedUserDefinedName", "voiceChannel", "internalIsArchived", "messageProtocol", "needsToDownloadRoles", "cellName", "teamRemoteIdentifier_data", "needsToCalculateUnreadMessages", "domain", "labels", "mutedStatus", "securityLevel", "lastServerTimeStamp", "hiddenMessages", "internalEstimatedUnreadSelfMentionCount", "needsToVerifyLegalHold", "lastReadServerTimeStamp", "teamRemoteIdentifier", "groupType", "lastUnreadMissedCallDate", "localMessageDestructionTimeout", "hasUnreadUnsentMessage", "draftMessageData", "lastServerSyncedActiveParticipants", "migratedToMLS", "primaryKey", "accessRoleStringsV2", "modifiedKeys", "mlsStatus", "isPendingInitialFetch", "epoch", "isDeletedRemotely", "needsToBeUpdatedFromBackend", "mlsGroupID", "lastModifiedDate", "nonTeamRoles", "isPendingMetadataRefresh", "allMessages", "hasReadReceiptsEnabled", "mlsVerificationStatus", "remoteIdentifier", "draftMessageNonce", "lastUnreadKnockDate", "language", "internalEstimatedUnreadCount", "isSelfAnActiveMember", "legalHoldStatus", "accessModeStrings", "ciphersuite", "silencedChangedTimestamp", "conversationType", "remoteIdentifier_data", "participantRoles", "privateChannelPermission", "clearedTimeStamp", "internalEstimatedUnreadSelfReplyCount", "accessRoleString", "isForcedReadOnly", "commitPendingProposalDate", "userDefinedName", "syncedMessageDestructionTimeout"]")

testThatItSnapshotsNilValues() - WireDataModelTests.SnapshotCenterTests

testThatItSnapshotsSetValues() - WireDataModelTests.SnapshotCenterTests - Details

Expand for error

 SnapshotCenterTests.swift:167: XCTAssertEqual failed: ("["participantRoles": 0, "allMessages": 1, "nonTeamRoles": 0, "labels": 0, "lastServerSyncedActiveParticipants": 0, "wireCellsMessageAttachmentDrafts": 0, "hiddenMessages": 0]") is not equal to ("["nonTeamRoles": 0, "allMessages": 1, "labels": 0, "hiddenMessages": 0, "lastServerSyncedActiveParticipants": 0, "participantRoles": 0]")

testThatSpecialKeysAreNotPartOfTheLocallyModifiedKeys - WireDataModelTests.ZMConversationTests - Details

Expand for error

 ZMConversationTests.m:285: ((conversation.keysTrackedForLocalModifications) equal to (expected)) failed: ("{(
     clearedTimeStamp,
     userDefinedName,
     archivedChangedTimestamp,
     cellName,
     lastReadServerTimeStamp,
     wireCellsMessageAttachmentDrafts,
     silencedChangedTimestamp,
     isForcedReadOnly
 )}") is not equal to ("{(
 ...

samwyndham · 2025-05-05T11:44:26Z

WireCells/Tests/WireCellsTests/WireCellsTests.swift

 import XCTest

-@testable @preconcurrency import WireCellsImplementation


Deleted these tests as WireCellsService no longer exists

jullianm

I've added some comments mostly about project structure since this PR is porting code from Android maybe there will be adjustments in future PRs to align to our codebase.

jullianm · 2025-05-05T13:37:18Z

WireCells/Sources/WireCellsAPI/WireCellsCellsRepository.swift

+    case failedToCreateWriteStream
+}
+
+public protocol WireCellsCellsRepository: Actor {


question: shouldn't this live in WireDomain with the other repositories ? Also the naming is strange, could it be simplified to WireCellsRepository ?

As far as I understand our goal is to move more of these kinds of things to feature modules. I don't think there is any benefit of having this in a shared module when it will only be used by wire cells

Regarding naming, I agree it is strange but I essentially it is saying that it is the CellsRepository and belonging to the WireCells feature.

Regarding naming, I took another look at this. In general I think Node is the better term than Cell as this is how things are referred to in the pydio API. Therefore I've renamed all the ...CellsCell... types. For example, WireCellsCellsRepositoryis nowWireCellsNodesRepository`

If it's as feature module, do we need prefix WireCells at all?

@dmitrysimkin It's a good point. Perhaps everything that is not public can be without the WireCells. I will leave as is for now and rename as I work more on the feature.

jullianm · 2025-05-05T13:49:32Z

WireCells/Sources/WireCellsAPI/WireCellsCellsAPI.swift

+    case missingData(String)
+}
+
+package protocol WireCellsCellsAPI: Sendable {


question: this looks like an API interface, should this be moved to WireAPI ? Also the DTO returned objects is something I haven't seen in codebase yet.
Since this code is ported from Android, I'm note sure if it's just a first PR and things will change to adapt to our codebase in next PRs or is it some new code structure we adopt.

Also should it be renamed to WireCellsAPI ?

Good point. The way WireCells interacts with backend is different then the other parts of our app - in fact it doesn't talk to wires backend at all. Instead it uses:

AWS SDK

Cells SDK which is network client generated from the swagger specs.

Therefore I don't think WireAPI is the right place for it. Regarding DTO, Thomas basically took everything as is including naming strategies from Android. This was done for speed and the idea that if we don't understand something we can ask the Android developers. Personally I'm in favor of merging first and then adopting as needed and unifying patterns with our code base.

Regarding naming I will ask in the developer channel for opinions.

jullianm · 2025-05-05T13:53:38Z

...alStore/WireCellsMessageAttachmentsDraftsLocalStore+WireCellsMessageAttachmentDraftDAO.swift

+import WireCellsAPI
+import WireDataModel
+
+extension WireCellsMessageAttachmentsDraftsLocalStore: WireCellsMessageAttachmentDraftDAO {


question: to be aligned with other local store stores protocol naming I guess it should be WireCellsMessageAttachmentDraftLocalStoreProtocol instead of WireCellsMessageAttachmentDraftDAO

I agree with you. But I'm in favor of merging soon and aligning terms in follow up PRs as we use these objects in practice and understanding better the DAO term. We are already using the concept of a DAO - see UpdateEventMigratorDAO but here that is the concrete implementation and the protocol is UpdateEventMigratorDAOProtocol.

So in summary, I'm in favor of aligning better with our exist codebase / future direction but would like to merge this PR quickly while spending some more time to think about these terms and probably discussing further in the architecture forum.

jullianm · 2025-05-05T13:56:53Z

WireCells/Sources/WireCellsAPI/Persistence/WireCellsMessageAttachmentDraftRepository.swift

+}
+
+// sourcery: AutoMockable
+public protocol WireCellsMessageAttachmentDraftRepository {


question: should this be moved to WireDomain as well?

I think the same answer here as your point about WireCellsCellsRepository

jullianm · 2025-05-05T13:59:31Z

WireCells/Sources/WireCellsAPI/WireCellsAwsClient.swift

+
+    /// Returns a pre-signed URL for the given S3 object key.
+    func getPreSignedUrl(objectKey: String) async throws -> String
+}


looks like another API interface, should it be moved to WireAPI (and probably renamed as well) ?

As mentioned about, I don't think so because we don't actually talk to the WireAPI. See comment above. If that changes we should perhaps move this to WireAPI but otherwise it is very specific to this package so I think it is best to keep it here.

dmitrysimkin · 2025-05-07T11:41:37Z

WireCells/Sources/WireCellsAPI/Model/WireCellsConversationID.swift

+    public var id: String {
+        "\(uuid.uuidString)@\(domain)"
+    }
+
+    public var pydioQualifiedID: String {
+        "\(uuid.uuidString)@\(domain)"
+    }


question: sanity check, shouldpydioQualifiedID be same as id ? if so you can use reuse id for pydioQualifiedID

dmitrysimkin · 2025-05-07T11:43:51Z

WireCells/Sources/WireCellsAPI/Model/WireCellsGetFilesResponseDTO.swift

+            // /!\ Will silently filter out nil values that could not be mapped to DTOs
+            nodes: nodes?.compactMap { $0.toDTO() } ?? []


question: is this smth that needs to be addressed now or later?

dmitrysimkin · 2025-05-07T13:09:42Z

WireCells/Sources/WireCellsAPI/Model/WireCellsNodeDTO.swift

+import CellsSDK
+package import Foundation
+
+package struct WireCellsNodeDTO: Equatable, Hashable, Sendable {


question: why do we need DTO? Usually DTOs is not pass between layers, is it case here?

dmitrysimkin · 2025-05-07T13:12:57Z

WireCells/Sources/WireCellsAPI/Persistence/WireCellsConversationDAO.swift

+}
+
+// sourcery: AutoMockable
+public protocol WireCellsConversationDAO {


question: Why is it DAO? Looks like a use case?

dmitrysimkin · 2025-05-07T13:15:03Z

WireCells/Sources/WireCellsAPI/Persistence/WireCellsMessageAttachmentDraftDAO.swift

+    func addAttachment(
+        uuid: UUID,
+        versionID: String,
+        conversationID: WireCellsConversationID,
+        mimeType: String,
+        fileName: String,
+        fileSize: UInt64,
+        dataPath: String,
+        nodePath: String,
+        uploadStatus: WireCellsAttachmentUploadStatus,
+        assetWidth: UInt64?,
+        assetHeight: UInt64?,
+        assetDuration: UInt64?
+    ) async throws(WireCellsMessageAttachmentDraftDAOError) -> WireCellsMessageAttachmentDraft


suggestion: too many params, would be nice introduce input model

dmitrysimkin · 2025-05-07T13:39:33Z

WireCells/Sources/WireCellsAPI/WireCellsNodesRepository.swift

+    case failedToCreateWriteStream
+}
+
+public protocol WireCellsNodesRepository: Actor {


issue: looks like too many methods in one repo, better split them into smaller ones, wdyt @samwyndham ?

dmitrysimkin · 2025-05-07T13:44:29Z

WireCells/Sources/WireCellsImplementation/Persistence/WireCellConversationDataSource.swift

+import Foundation
+import WireCellsAPI
+
+final class WireCellConversationDataSource: WireCellsNodeConversationRepository {


suggestion: naming is not consistent, e.g. we have Implementation for Client WireCellsAWSClientImplementation: WireCellsAWSClient but for repository protocol implementation we have DataSource. can we make it consistent?

dmitrysimkin · 2025-05-07T14:01:14Z

WireCells/Sources/WireCellsImplementation/WireCellsAwsClientImplementation.swift

+    private enum Constants {
+        static let bucket = "io"
+        static let multipartChunkSize = 10 * 1024 * 1024
+        static let maxRegularUploadSize = 100 * 1024 * 1024
+        static let preSignedUrlExpiryInHours = 24
+        static let readAsyncChunkSize = 5 * 1024 * 1024
+        static let region = "us-east-1"
+    }


praise : for adding constants

jullianm

looks good overall, approving to unblock future tasks on WireCells, my assumption though is that things will be adjusted a little architecture wise as the feature moves forward

El-Fitz added 2 commits April 16, 2025 15:34

feat: port most Android kalium models & interfaces

a71c332

feat: implement persistency for conversations attributes, boilerplate…

34e09e9

… for drafts attachments

El-Fitz self-assigned this Apr 18, 2025

El-Fitz added 2 commits April 21, 2025 19:52

feat: port & implement S3 client & upload manager

ef217d8

style: fix formatting

d61ba98

mathias-niboulies requested a review from samwyndham April 22, 2025 08:41

mathias-niboulies assigned samwyndham Apr 22, 2025

samwyndham unassigned El-Fitz Apr 29, 2025

samwyndham added 22 commits April 30, 2025 13:12

Comment out non-compiling code

0af8aad

Fix `MessageAttachmentDraftDataSource.add(conversationID:node:mimeTyp…

013ed95

…e:dataPath:metadata:uploadStatus:)` compile error

Comment out non-compiling code

a195a25

Fix compile issue

a8a9565

Comment broken tests

67ff48a

Perhaps we can delete these but I will comment them for the time being

Make Aws uppercase

7052223

Change cellsApi to cellsAPI

588329b

Change Dao to DAO

60d1f2e

Fix WireCellsCellsDataSource concurrency issues

2004156

Rename Dto to DTO

27ece32

Fix file naming

7b4d9e1

Remove TODO

5d4d55c

It is not clear to me what this TODO means.

Delete stray commented code

810e124

Make `WireCellsMessageAttachmentDraftRepository.observe(conversationI…

2cc64e2

…D:)` async

Implement WireCellsAWSClientImplementation.getPreSignedUrl(objectKey:)

2c852ab

Report download progress

f5bea45

Update cells SDK

6493154

Create todo comment

95b68f0

Merge branch 'develop' into feat/wire-cells-upgrade-service

1bbb808

# Conflicts: # wire-ios-data-model/Resources/zmessaging.xcdatamodeld/zmessaging2.125.0.xcdatamodel/contents # wire-ios-data-model/WireDataModel.xcodeproj/project.pbxproj

Store upload status as Int16

7804585

Create zmessaging2.126.0 model

a53ed76

This commit doesn't contain any new/updated entities - just the new model

Update data model schema

9140363

samwyndham added 5 commits May 2, 2025 16:10

Commit auto generated mocks

e9f79d1

Fix core data not finding WireCellsMessageAttachmentDraftEntity class

5752bbd

Test creating a WireCellsMessageAttachmentDraftEntity

5408260

Delete unused tests

797adab

Lint & format

654a478

samwyndham marked this pull request as ready for review May 2, 2025 15:30

samwyndham added 3 commits May 5, 2025 09:40

Fix compile errors

adef6d1

Update Databases changelog

47b501a

Fix DatabaseMigrationTests

9cbbe26

samwyndham reviewed May 5, 2025

View reviewed changes

samwyndham approved these changes May 5, 2025

View reviewed changes

samwyndham requested review from a team, netbe, caldrian and jullianm and removed request for a team, netbe and caldrian May 5, 2025 11:47

jullianm reviewed May 5, 2025

View reviewed changes

samwyndham added 3 commits May 7, 2025 10:57

Rename Cell types to Node

bda2386

Rename Cell types to Node in file names

9b8a4fb

Rename WireCellsNodesApiImpl to WireCellsNodesAPIImpl

bd5f9f1

samwyndham requested review from jullianm and dmitrysimkin and removed request for dmitrysimkin May 7, 2025 09:05

Merge branch 'develop' into feat/wire-cells-upgrade-service

11f060d

dmitrysimkin reviewed May 7, 2025

View reviewed changes

jullianm approved these changes May 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: port Wire Android services & use cases - WPB-16148 #2920

feat: port Wire Android services & use cases - WPB-16148 #2920

El-Fitz commented Apr 18, 2025 •

edited by samwyndham

Loading

github-actions bot commented May 2, 2025 •

edited

Loading

datadog-wireapp bot commented May 2, 2025 •

edited

Loading

samwyndham May 5, 2025

jullianm left a comment

jullianm May 5, 2025

samwyndham May 7, 2025

samwyndham May 7, 2025 •

edited

Loading

dmitrysimkin May 7, 2025

samwyndham May 7, 2025

jullianm May 5, 2025

samwyndham May 7, 2025

jullianm May 5, 2025

samwyndham May 7, 2025 •

edited

Loading

jullianm May 5, 2025

samwyndham May 7, 2025

jullianm May 5, 2025

samwyndham May 7, 2025

dmitrysimkin May 7, 2025

dmitrysimkin May 7, 2025

dmitrysimkin May 7, 2025

dmitrysimkin May 7, 2025

dmitrysimkin May 7, 2025

dmitrysimkin May 7, 2025

dmitrysimkin May 7, 2025

dmitrysimkin May 7, 2025

jullianm left a comment

		import XCTest

		@testable @preconcurrency import WireCellsImplementation

		// /!\ Will silently filter out nil values that could not be mapped to DTOs
		nodes: nodes?.compactMap { $0.toDTO() } ?? []

feat: port Wire Android services & use cases - WPB-16148 #2920

Are you sure you want to change the base?

feat: port Wire Android services & use cases - WPB-16148 #2920

Conversation

El-Fitz commented Apr 18, 2025 • edited by samwyndham Loading

Issue

Remaining

Testing

Checklist

github-actions bot commented May 2, 2025 • edited Loading

Test Results

datadog-wireapp bot commented May 2, 2025 • edited Loading

Datadog Report

❌ Failed Tests (4)

Choose a reason for hiding this comment

jullianm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samwyndham May 7, 2025 • edited Loading

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

samwyndham May 7, 2025 • edited Loading

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

Choose a reason for hiding this comment

jullianm left a comment

Choose a reason for hiding this comment

El-Fitz commented Apr 18, 2025 •

edited by samwyndham

Loading

github-actions bot commented May 2, 2025 •

edited

Loading

datadog-wireapp bot commented May 2, 2025 •

edited

Loading

samwyndham May 7, 2025 •

edited

Loading

samwyndham May 7, 2025 •

edited

Loading