Migrating to Room Persistence

guardianproject / haven

Haven is for people who need a way to protect their personal spaces and possessions without compromising their own privacy, through an Android app and on-device sensors

https://guardianproject.github.io/haven/

GNU General Public License v3.0

6.62k stars 728 forks source link

Migrating to Room Persistence #307

Closed archie94 closed 5 years ago

archie94 commented 6 years ago

Hello,

I have included Room persistence library for db operations in this project, removing the dependency of Sugar ORM. Have tested this on my device and seems to hold up well, although I would like to know best testing procedures before this feature can be merged.

Event and EventTrigger are now Room entities instead of SugarRecord.
Added dependency for Kotlin.
DB schema are the same; db version not increased and migrations should not be necessary.

Kindly review this PR and let me know your thoughts.

What can be improved

right now db queries are carried out on the main thread. Ideally we should do this asynchronously.

Aims to close #129

archie94 commented 6 years ago

@n8fr8 @lukeswitz your views on this?

Also, any leads on why CI check is failing?

lukeswitz commented 6 years ago

The fdroid server repo needs to be force updated from the looks of it; @n8fr8 is the one with access to that. Artifacts get stuck in Travis builds sometimes and can be the culprit.

From the CI run log :

The following packages have unmet dependencies:
 fdroidserver : Depends: python3-docker but it is not installable
                Depends: androguard (>= 3.1.0~rc2) but it is not installable
E: Unable to correct problems, you have held broken packages.

n8fr8 commented 6 years ago

Any thoughts on a migration path from Sugar to this? We have people with existing installs who might upgrade, and it would be nice to handle that case. All their old media is stored externally, so perhaps we just say "Looking for your old media, you cand find it here" ?

n8fr8 commented 6 years ago

Reviewing this some more, and Kotlin... hmm. I know it is a thing, and great that you are using it. I don't know if I am ready to have our whole persistence layer rewritten in it yet.

Any thoughts form others on this? @lukeswitz ?

archie94 commented 6 years ago

Have tested migrations from Sugar database version 3 to this current implementation and it works. Will test for previous db versions too. My aim is to make this transition to Room seamless and it should not create any increased user frustration. Improving clarity on where the recorded data is stored is however not the aim of this PR.

Regarding Kotlin I am interested to know what others have to say. My view is Kotlin produces smaller codebase which may be easier to maintain in the long run.

lukeswitz commented 6 years ago

Kotlin, like Swift, has advantages but not when contributing to a large project written entirely in Java or Obj-C.

Even though it’s new and fast, the language is total beta like Swift- frequent updates to maintain that code is more work than using Java. That’s just my $.02, there’s nothing bad about it but nothing good either.

What do people need to do in order for the migration to succeed in onboarding? What happens when the process is interrupted?

n8fr8 commented 6 years ago

@archie94 I appreciate your continued work on this, and will review and consider

archie94 commented 6 years ago

Thanks!

I have already started the work of moving all DB queries on background thread. There are a few features that need to be wrapped up before this can be closed

adding tests for the DB migrations. Seems to be the only way to verify that migration will always succeed.
adding a message for the user when the migration is in progress.

archie94 commented 5 years ago

Update:

added tests for Database migration.
added a message for the user while the migration is in process.

archie94 commented 5 years ago

I am thinking of adding a JobService for regular removal of deleted log files from disk instead of instant cleanup of files on Event delete. For supporting legacy OS versions Firebase Job Dispatcher seems to be the way forward. However, it depends on GooglePlayServices and I understand a lot of folks do not wish to be bound to this.

@lukeswitz @n8fr8 your thoughts on these are appreciated.

archie94 commented 5 years ago

Looking into android-job which looks promising and does not depend on Play Services.

archie94 commented 5 years ago

My work seems done. Waiting for your review @n8fr8

This is a debug apk and for builds based on the current master branch, all log data should be preserved. For anyone willing to give the app a spin, download apk from https://github.com/archie94/haven/releases/tag/v0.2.0-alpha4-room

Do report any bug/suggestions you find. :)