Properly support multiple repositories with the same app
Fixes #511 (closed).
Provides proper support for multiple repos in the database + updater (despite not being able to reorder them in the UI yet).
Info
Previously, each app had only one row in the app table. This caused problems when its metadata was updated, because it would get overriden if two repositories provided the same app. This change:
- Creates a new
packagetable which acts as the table that includes one row for each app (like theapptable did previously) - Re-purposes the existing
apptable as anapp_metadatatable, where each package can have multiple rows (one for each repository that provides that package)
This results in some queries which are slightly more complex, but the overall performance should not change too much. In the long term, it should have a net positive impact on performance, because we no longer need to join between tables based on a package name. Now we almost exclusively use integer IDs to join between tables. There are also appropriate indexes which make the queries avoid full table scans in all cases I'm aware of.
I realise this is a big MR, but it is as small as I could make it without submitting half finished stuff which breaks master. I've done my best to merge smaller MRs before hand, but I was unable to identify anything else to pull out of this MR as a separate thing.
Migration
All app/apk metadata is dropped, and then the repos are asked to update themselves again next time F-Droid starts.
Additionally, existing repositories have their priority changed to something more meaningful. On current master, if you add two custom repositories in addition to the four default ones, you will get the following:
| rowid | address | priority |
|---|---|---|
| 1 | https://f-droid.org/repo | 10 |
| 2 | https://f-droid.org/archive | 20 |
| 3 | https://guardianproject.info/fdroid/repo | 10 |
| 4 | https://guardianproject.info/fdroid/archive | 20 |
| 5 | http://10.0.0.6:8888/normal-repo/repo | 10 |
| 6 | http://10.0.0.6:8888/conflicting-repo/repo | 10 |
Note how the priority defaults to 10 for each additional repository which is added. This MR should change the priorities so they look like so:
Here is a snipped from the logcat from DBHelper during the migration:
Setting priority of repo https://f-droid.org/repo to 1
Setting priority of repo https://f-droid.org/archive to 2
Setting priority of repo https://guardianproject.info/fdroid/repo to 3
Setting priority of repo https://guardianproject.info/fdroid/archive to 4
Setting priority of repo http://10.0.0.6:8888/normal-repo/repo to 5
Setting priority of repo http://10.0.0.6:8888/conflicting-repo/repo to 6All newly added repositories on this branch will get the next highest available priority.
Future work
One thing which is not yet supported fully: Suggested version code.
Two repositories can end up with the same exact .apk file. If that .apk is the "suggested version", then we should eliminate the idea of "suggested version code" and instead have a "suggested apk" (which implicitly includes the repository it comes from, so we choose the one with the better priority). Right now, we kind of assume that it doesn't matter which repo provides the suggested apk, as long as one of them has an .apk with th ecorect version code and signing key.
I guess it doesn't particularly matter from a security perspective, because a malicious repo wont be able to trick a user into installing an apk with a different signing key, but it would be good to iron this out.
Merge request reports
Activity
mentioned in issue #33 (closed)
Added 23 commits:
- bae6f328...943e0d0a - 10 commits from branch
fdroid:master - 389a52ea - Added package table.
- 05bff3d5 - WIP: Making metadata table work. Requires significant refactorings throughout :(
- 075a8b2a - Enable multirepo tests, make them pass.
- debea094 - Moved tests into updater package, updated multiRepo.*.jar repos.
- 51216210 - Add test for repo priorities + app metadata. Not passing yet.
- 3788a9f5 - Finalise tests for repo priorities + app metadata
- 3883d332 - Appease checkstyle + pmd
- 412099eb - Migrate repo priorities safely.
- c3f53ebb - Reassign priorities of repos upon upgrade.
- 1c776dec - Clarify a limitation in the current implementation
- 600b65a8 - Cleaning up/commenting AppProvider
- 202b0a0d - Update priorities for default repos to go from 1-4 instead of 10 + 20
- d865f302 - Clarify some of the database stuff around database providers.
Toggle commit list- bae6f328...943e0d0a - 10 commits from branch
Added 27 commits:
- d865f302...20a5f423 - 13 commits from branch
fdroid:master - 05991efd - Move code causing verify error into separate helper class
- e440d5ce - Added package table.
- 512ed830 - WIP: Making metadata table work. Requires significant refactorings throughout :(
- 76a15990 - Enable multirepo tests, make them pass.
- c4eb83c6 - Moved tests into updater package, updated multiRepo.*.jar repos.
- 8df4fb45 - Add test for repo priorities + app metadata. Not passing yet.
- 8176b93f - Finalise tests for repo priorities + app metadata
- 24b39758 - Appease checkstyle + pmd
- 42e802a0 - Migrate repo priorities safely.
- 3559afe6 - Reassign priorities of repos upon upgrade.
- a1d24ce3 - Clarify a limitation in the current implementation
- e37aa683 - Cleaning up/commenting AppProvider
- 616cbea2 - Update priorities for default repos to go from 1-4 instead of 10 + 20
- 7bdee52b - Clarify some of the database stuff around database providers.
Toggle commit list- d865f302...20a5f423 - 13 commits from branch
Added 55 commits:
- 7bdee52b...b5b5dd4e - 42 commits from branch
fdroid:master - 41b83a05 - Added package table.
- c741a4d8 - WIP: Making metadata table work. Requires significant refactorings throughout :(
- 852d9f22 - Enable multirepo tests, make them pass.
- 7a026791 - Moved tests into updater package, updated multiRepo.*.jar repos.
- 88d5ee12 - Add test for repo priorities + app metadata. Not passing yet.
- cd7573fa - Finalise tests for repo priorities + app metadata
- f2605a50 - Appease checkstyle + pmd
- 63fa8f72 - Migrate repo priorities safely.
- 1044f490 - Reassign priorities of repos upon upgrade.
- 578332ae - Clarify a limitation in the current implementation
- 4c7d510f - Cleaning up/commenting AppProvider
- db789272 - Update priorities for default repos to go from 1-4 instead of 10 + 20
- 4aed3e1c - Clarify some of the database stuff around database providers.
Toggle commit list- 7bdee52b...b5b5dd4e - 42 commits from branch
Added 61 commits:
- 4aed3e1c...c8182d9c - 42 commits from branch
fdroid:master - 3e65c2cf - Added package table.
- 6674f6a7 - WIP: Making metadata table work. Requires significant refactorings throughout :(
- 055c712e - Enable multirepo tests, make them pass.
- d0bd8074 - Moved tests into updater package, updated multiRepo.*.jar repos.
- de1d2fd9 - Add test for repo priorities + app metadata. Not passing yet.
- 81d33beb - Finalise tests for repo priorities + app metadata
- c446060c - Appease checkstyle + pmd
- 97dbc3e7 - Migrate repo priorities safely.
- bb465785 - Reassign priorities of repos upon upgrade.
- 3063a633 - Clarify a limitation in the current implementation
- 957ba43f - Cleaning up/commenting AppProvider
- e717c94e - Update priorities for default repos to go from 1-4 instead of 10 + 20
- 77c27660 - Clarify some of the database stuff around database providers.
- 9cd2ee96 - Added helper function for debugging SQL queries during development
- c899f064 - Precalculate the preferred metadata, rather than always at runtime
- 8aa843e3 - Moved regression test to appropriate package.
- fd014484 - WIP: Making correct apks get found when updating repo.
- 6d2e2f6e - Clarify that sometimes we don't know which repos apk we are asking for.
- 4615386b - Fixes after rebasing
Toggle commit list- 4aed3e1c...c8182d9c - 42 commits from branch
Milestone changed to %0.102
Okay, I realise this is a monster changeset, but I have struggled to keep the size down (by the earlier collection of smaller MRs in August). One thing this has going for it is that:
- It has good test coverage which should all pass.
- We are only in the earliest possible stages of 0.102, so have time to find out if anything is particularly troublesome.
Could I suggest that any potential review focuses strongly on the tests that have been added to ensure that they are indeed testing the correct thing? That way, it is a pretty good sign that the actual code changes are doing as they expect.
Mentioned in issue #775 (closed)
Mentioned in merge request !400 (merged)
Mentioned in merge request !401 (merged)
Added 21 commits:
- 6c462713 - Renamed generic sounding methods to be more specific.
- 97cf6934 - When inserting a new repo, assign the priority appropriately.
- 45f9379f - Added helper function for debugging SQL queries during development
- 486e8e69 - Cleanup DBHelper in prep for package table in the future.
- 88c536ef - Fix incorrect version check in db helper.
- 5efa53b4 - Added package table.
- 44a82c4a - Migrate priorities safely.
- 8ed88488 - WIP: Making metadata table work. Requires significant refactorings throughout :(
- 6d2fdb28 - Enable multirepo tests, make them pass.
- e25d26ac - Moved tests into updater package, updated multiRepo.*.jar repos.
- 1d1c1ebb - Add test for repo priorities + app metadata. Not passing yet.
- 3ec64d6d - Finalise tests for repo priorities + app metadata
- efdf328f - Clarify a limitation in the current implementation
- c08a2a7b - Cleaning up/commenting AppProvider
- f2a58ad6 - Update priorities for default repos to go from 1-4 instead of 10 + 20
- 01b8f7f4 - Clarify some of the database stuff around database providers.
- ab02058e - Precalculate the preferred metadata, rather than always at runtime
- 2cc15535 - Moved regression test to appropriate package.
- 3a24d21f - WIP: Making correct apks get found when updating repo.
- d062af09 - Clarify that sometimes we don't know which repos apk we are asking for.
- e0a1d238 - Appease checkstyle + pmd
Toggle commit list-
I managed to pull out a little bit into !400 (merged) and !401 (merged), hopefully making this a tiny bit more reasonable. Here is the sum total of those two MRs that was essentially pulled verbatim out of this:
app/src/main/java/org/fdroid/fdroid/AppDetails.java | 8 ++++---- app/src/main/java/org/fdroid/fdroid/RepoUpdater.java | 4 ++-- app/src/main/java/org/fdroid/fdroid/UpdateService.java | 2 +- app/src/main/java/org/fdroid/fdroid/Utils.java | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java | 74 ++++++++++++++++++++++++++++++++++---------------------------------------- app/src/main/java/org/fdroid/fdroid/data/AppProvider.java | 12 ++++++++---- app/src/main/java/org/fdroid/fdroid/data/DBHelper.java | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------- app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java | 21 ++++++++++++++------- app/src/main/java/org/fdroid/fdroid/data/Schema.java | 14 ++++++++++++++ app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java | 4 ++-- app/src/main/java/org/fdroid/fdroid/installer/Installer.java | 2 +- app/src/main/java/org/fdroid/fdroid/privileged/views/InstallConfirmActivity.java | 2 +- app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java | 8 ++++---- app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java | 2 +- app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java | 10 +++++----- app/src/test/java/org/fdroid/fdroid/data/ProviderUriTests.java | 4 ++-- 16 files changed, 239 insertions(+), 85 deletions(-)
