Skip to content

Use an integer primary key to join `fdroid_app` and `fdroid_apk` rather than the apps package name.

Disclaimer:

I realise this is a big change, but it needs to be done at some point, and it is not amenable to smaller changes, due to the fact that the app/apk relationship is so ingrained throughout F-Droid. Luckily, we have really quite comprehensive test coverage of the F-Droid ContentProviders which helps to confirm that nothing should be majorly broken here.

Some points of note:

This is the first part of implementing #511 (closed), whereby the DB is refactored to better support multiple repositories.

Instead of joining fdroid_app and fdroid_apk tables using the package name, join based on an integer id autogenerated by sqlite. By default sqlite calls this rowid and it exists for every table, unless you've specified your own NUMBER AUTO INCREMENT PRIMARY KEY field. We have not done this for fdroid_app, so rowid is indeed the key we use in this MR. The package name was previously id in both the app and apk tables. Now fdroid_app makes use of rowid and fdroid_apk has a foreign key called appId.

The ApkProvider used to get away with only really querying the fdroid_apk table, and thus it didn't have to prefix any of the field names in the query with the table name. However now it always joins onto the fdroid_app table also, and as such, there are many places where field names needed to be prefixed with the table name (e.g. the apk alias or the app alias) to ensure the SQL is unambiguous when fields with the same name exist in both tables. The catch is, we want to reuse helper functions that build fragments of SQL, such as "Query based on package name". These helper functions are used both when updating and deleting apks (where field table prefixes are not allowed) and also in select statements (where they are required). Thus this changes comes with an includeTableAlias argument added to many of these methods (e.g. ApkProvider.queryApp).

There is still a package name column in the fdroid_apk table (the id field). This will be removed in future MRs and replaced with the package name from the joined fdroid_app table.

The RepoPersister used to dump apps in the db, then dump apks into the db. Now it needs to be a bit more nuanced, and dump apps into the db, then ask the db what rowid was assigned to the apps. This is then used when dumping the apks into the db. This also required some changes to how the TempAppProvider and AppProvider interact. In the interests of reusing code, both of these are able to provide operations on a similarly structured table but one is an in memory table (temp_fdroid_app) and the other is on disk (fdroid_app). In the past this was simpler, because the only interaction with the TempAppProvider was by using lists of ContentOperations. Whereas now that we need to ask more substantial questions of the TempAppProvider other than "Insert this thing" or "update that thing", we needed to implement the query method in TempAppProvider similar to how it is in the base class AppProvider. As such, the common code for the base class and subclass query methods was extracted into AppProvider.runQuery().

I tried to minimize the changes to the test suite as much as possible, so that it is possible to verify that they pass under the same conditions as before this change. However some changes were required to support the notion that apks depend on an app and its rowid, whereas this was not the case before. Thus there is some more boilerplate in the tests to ensure that inserting an apk ensures an app entry is present in the db too.

Merge request reports