-
Notifications
You must be signed in to change notification settings - Fork 784
Feature/bca/migrate rust in db #8405
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
Feature/bca/migrate rust in db #8405
Conversation
bb31a2d
to
f1036d4
Compare
d13521f
to
5e28631
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment about the migration, else LGTM
migrateOperation.dynamicExecute(realm, rustDirectory, rustEncryptionConfiguration.getDatabasePassphrase()) | ||
|
||
// wa can't delete all for now, but we can do some cleaning | ||
realm.schema.get("OlmSessionEntity")?.transform { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you must also delete the file OlmSessionEntity
to be able to do that, because else it will be added again to the schema.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added an assertion to the test to ensure that entities are removed. But yeah we keep the table for now (albeit empty)
SonarCloud Quality Gate failed. |
Type of change
Content
`Perform the legcay to rust migration by using a new CryptoDB migeration (
MigrateCryptoTo022
), instead of in the DI layer.Main change is that the former code was migrating a
Realm
instance, but now as it's in a migration it has to do it with aDynamicRealm
. I kept the former code as a utility (could be deleted?), but only the dynamic migration is used now.Some of the database migration test have been updated as the rust migration is now down seamlessly.
Notice that now, reverting from rust to kotlin crypto will fail due to incompatible db. Anyhow such back migration is not technically possible atm due to lack of support in vodozemac. (will fail with
Illegal Argument: Provided schema version 21 is less than last set version 22.
)Motivation and context
Screenshots / GIFs
Tests
Tested devices
Checklist