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

feat: add initial crossplatform backup serialization [WPB-10575] #3121

Conversation

vitorhugods
Copy link
Member

@vitorhugods vitorhugods commented Nov 22, 2024

StoryWPB-10575 Cross Platform Backup: Write common backup / restore library


PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

This PR adds initial serialization/deserialization to the backup module.

Solutions

Using Protobuf, and a common implementation for JS and Non-JS.

The main difference between JS and Non-JS, is that:

  • JS doesn't have 64-bit integers (a.k.a. Long) for date/millis
  • JS/browser doesn't deal with "files". We can't write to paths. JS loads everything into memory and deals with byte arrays. Android and iOS need to be more careful with this and avoiding having everything in memory, so dealing with data streams is better.

Dependencies

Needs releases with:

Testing

Test Coverage

  • I have added automated test to this contribution

PR Post Merge Checklist for internal contributors

  • If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.

References
  1. https://sparkbox.com/foundry/semantic_commit_messages
  2. https://github.com/wireapp/.github#usage
  3. E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

This commit introduces a new cross-platform backup module to the project with support for iOS, Web, and Android. Additionally, it enables JavaScript multiplatform support in existing `kalium/network-model` and `kalium/data` projects.
This commit introduces a new multiplatform backup feature including export and import capabilities. It adds data classes, serialization, tests, and Gradle dependencies to support cross-platform backup data handling, including proper ProtoBuf serialization.
Copy link
Contributor

github-actions bot commented Nov 22, 2024

Test Results

3 309 tests   3 202 ✅  5m 1s ⏱️
  566 suites    107 💤
  566 files        0 ❌

Results for commit bc8ddeb.

♻️ This comment has been updated with latest results.

@datadog-wireapp
Copy link

datadog-wireapp bot commented Nov 22, 2024

Datadog Report

Branch report: feat/add-initial-crossplatform-backup-serialization
Commit report: 8227a80
Test service: kalium-jvm

✅ 0 Failed, 3202 Passed, 107 Skipped, 34.91s Total Time

@vitorhugods vitorhugods marked this pull request as ready for review November 22, 2024 12:25
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (epic/multiplatform-backup@1a57f5d). Learn more about missing BASE report.

Additional details and impacted files
@@                     Coverage Diff                      @@
##             epic/multiplatform-backup    #3121   +/-   ##
============================================================
  Coverage                             ?   54.03%           
============================================================
  Files                                ?     1246           
  Lines                                ?    36152           
  Branches                             ?     3656           
============================================================
  Hits                                 ?    19534           
  Misses                               ?    15207           
  Partials                             ?     1411           

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1a57f5d...bc8ddeb. Read the comment docs.

---- 🚨 Try these New Features:


import kotlin.js.JsExport

@JsExport
Copy link
Member Author

Choose a reason for hiding this comment

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

@JsExport is needed so this class can be called from JS later.

Comment on lines +94 to +97
expect class BackupDateTime

expect fun BackupDateTime(timestampMillis: Long): BackupDateTime
expect fun BackupDateTime.toLongMilliseconds(): Long
Copy link
Member Author

Choose a reason for hiding this comment

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

This is because JS doesn't have 64-bit Integers (Long).
So we can use Long for iOS/Android, and a custom Date wrapper for JS.

Comment on lines +57 to +70
@ObjCName("add")
fun addUser(user: BackupUser) {
allUsers.add(user)
}

@ObjCName("add")
fun addConversation(conversation: BackupConversation) {
allConversations.add(conversation)
}

@ObjCName("add")
fun addMessage(message: BackupMessage) {
allMessages.add(message)
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I was planning on having add for all three methods, only changing the parameter.

Unfortunately it doesn't work on JS, as it is not a typed language...

So at least when calling from Swift it will look like this:

backupExporter.add(user: BackupUser(...))
backupExporter.add(conversation: BackupConversation(...))
backupExporter.add(message: BackupMesage(...))

BackupMessageContent.Text(proContent.value.content)
}

null -> TODO()
Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines +24 to +31
override fun equals(other: Any?): Boolean {
if (this === other) return true
if (other == null || this::class.js != other::class.js) return false

other as BackupDateTime

return this.toLongMilliseconds() == other.toLongMilliseconds()
}
Copy link
Member Author

Choose a reason for hiding this comment

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

For some reason backupDateTime == backupDateTime doesn't work on JS, so I had to do it myself like so.


@JsExport
sealed class BackupMessageContent {
data class Text(val text: String) : BackupMessageContent()

Choose a reason for hiding this comment

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

Text messages also contain link previews, mentions, and are possibly a reply to another message. Are we planning to include these in the first version?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can expand later, like what's done in:


@OptIn(ExperimentalStdlibApi::class)
@ShouldRefineInSwift // Hidden in Swift
fun serialize(): ByteArray {

Choose a reason for hiding this comment

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

Is this the method we call to produce the backup? The comment suggests it's hidden but it the POC it is visible but returns a KotlinByteArray which I'm guessing I would create a file with and present it to the user. Though I'm concerned about the size of this data and loading it all into memory at once.

I'm wondering if it would be possible for the client to pass a URL to which the library could write the backup file to. This could allow for more memory efficienct handling of the backup data.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed :)
The goal is to implement the byte-reading part in the commonMain code, which will be used internally for all clients.

JS will only have this bytearray alternative.

Android and iOS will be able to pass paths that will stream and take paginated chunks of data in, without having to load the whole file into memory

This PR only handles initial serialization. There are more PRs incoming with:

  • Expanded serialization
  • Encryption/Decryption
  • Creating APIs specific for dealing with files on iOS and Android
  • Solidifying/tweaking the API

@echoes-hq echoes-hq bot added the echoes: product-roadmap/feature Work contributing to adding a new feature as part of the product roadmap. label Nov 28, 2024
@vitorhugods vitorhugods merged commit ccb8193 into epic/multiplatform-backup Dec 2, 2024
20 checks passed
@vitorhugods vitorhugods deleted the feat/add-initial-crossplatform-backup-serialization branch December 2, 2024 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
echoes: product-roadmap/feature Work contributing to adding a new feature as part of the product roadmap. 👕 size: XL type: feature ✨
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants