Right now there is only one test in there anyway, so hopefully this is
a good tradeoff in terms of our time wasted vs not being able to run
those tests.
Moved index adding to a helper function, so that the same mistake isn't made
again. That is, indexes should be the same whether upgrading or creating a
database. Thus, the code to add indexes should always be the same regardless
of the reason for an index being added. The `IF NOT EXISTS` syntax helps
to allow the same queries to add during creatin and migration of database.
The fact that sqlite chose to do a "FULL TABLE SCAN" right off the bat when showing a list
of apps results in it having to do extra work at the end of the query to sort. If the scan
can be utilise an index, then the sorting is already done and a b-tree need not be constructed.
Thus, this introduces indexes for both the `name` column (used to sort most lists of apps)
and the `added` column (used to figure out the `Whats New` apps).
This is required because SQLite "rowids" are 64 bit integers:
> all rows within SQLite tables have a 64-bit signed integer
> key that uniquely identifies the row within its table."
https://sqlite.org/lang_createtable.html#rowid
This is important for the ability to refactor the database for better performance in the future.
See #511 for details.
For those interested in the details, here is a query plan of selecting the "All" category of apps before
this commit:
* `SCAN TABLE fdroid_app USING INDEX app_id`
* `SEARCH TABLE fdroid_apk USING INDEX apk_id (id=?)`
* `SEARCH TABLE fdroid_repo USING INTEGER PRIMARY KEY (rowid=?)`
* `SEARCH TABLE fdroid_installedApp AS installed USING INDEX sqlite_autoindex_fdroid_installedApp_1 (appId=?)`
* `SEARCH TABLE fdroid_apk AS suggestedApk USING INDEX sqlite_autoindex_fdroid_apk_1 (id=? AND vercode=?)`
* `USE TEMP B-TREE FOR ORDER BY`
And here is a query plan of afterwards:
* `SCAN TABLE fdroid_app`
* `SEARCH TABLE fdroid_apk USING INDEX apk_appId (appId=?)`
* `SEARCH TABLE fdroid_repo USING INTEGER PRIMARY KEY (rowid=?)`
* `SEARCH TABLE fdroid_installedApp AS installed USING INDEX sqlite_autoindex_fdroid_installedApp_1 (appId=?)`
* `SEARCH TABLE fdroid_apk AS suggestedApk USING INDEX apk_appId (appId=?)`
* `USE TEMP B-TREE FOR ORDER BY`
The things of note are:
* `SCAN TABLE` doesn't use an index, which means that it is really using the rowid index. Shouldn't behave much differently.
* The second item now uses an integer primary key index rather than a package name index. Should increase search speed marginally which was the goal of this commit. As more apks exist, the speed improvement will also increase.
Removed `TargetApi` annotation from class, pushing it to methods, and fix API problems.
Fixes#708.
On gingerbread (in my case, 2.3.4) F-Droid will reliably crash whenever it views `AppDetails` with the following error:
```
E java.lang.NoSuchMethodError: android.content.Context.getDrawable
E at org.fdroid.fdroid.privileged.views.AppSecurityPermissions$MyPermissionGroupInfo.loadGroupIcon(AppSecurityPermissions.java:111)
E at org.fdroid.fdroid.privileged.views.AppSecurityPermissions$PermissionItemView.setPermission(AppSecurityPermissions.java:158)
E at org.fdroid.fdroid.privileged.views.AppSecurityPermissions.getPermissionItemView(AppSecurityPermissions.java:396)
E at org.fdroid.fdroid.privileged.views.AppSecurityPermissions.displayPermissions(AppSecurityPermissions.java:370)
E at org.fdroid.fdroid.privileged.views.AppSecurityPermissions.getPermissionsView(AppSecurityPermissions.java:348)
E at org.fdroid.fdroid.AppDetails$AppDetailsSummaryFragment.buildPermissionInfo(AppDetails.java:1381)
E at org.fdroid.fdroid.AppDetails$AppDetailsSummaryFragment.setupView(AppDetails.java:1348)
E at org.fdroid.fdroid.AppDetails$AppDetailsSummaryFragment.onCreateView(AppDetails.java:1095)
E at android.support.v4.app.Fragment.performCreateView(Fragment.java:2074)
E at android.support.v4.app.FragmentManagerImpl.moveToState(FragmentManager.java:1104)
E at android.support.v4.app.FragmentManagerImpl.moveToState(FragmentManager.java:1286)
E at android.support.v4.app.BackStackRecord.run(BackStackRecord.java:758)
E at android.support.v4.app.FragmentManagerImpl.execPendingActions(FragmentManager.java:1671)
E at android.support.v4.app.Fragment.performStart(Fragment.java:2096)
E at android.support.v4.app.FragmentManagerImpl.moveToState(FragmentManager.java:1142)
E at android.support.v4.app.FragmentManagerImpl.moveToState(FragmentManager.java:1286)
E at android.support.v4.app.FragmentManagerImpl.moveToState(FragmentManager.java:1268)
E at android.support.v4.app.FragmentManagerImpl.dispatchStart(FragmentManager.java:2148)
E at android.support.v4.app.FragmentController.dispatchStart(FragmentController.java:212)
E at android.support.v4.app.FragmentActivity.onStart(FragmentActivity.java:625)
E at org.fdroid.fdroid.AppDetails.onStart(AppDetails.java:424)
E at android.app.Instrumentation.callActivityOnStart(Instrumentation.java:1129)
E at android.app.Activity.performStart(Activity.java:3791)
E at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:1620)
E at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:1663)
E at android.app.ActivityThread.access$1500(ActivityThread.java:117)
E at android.app.ActivityThread$H.handleMessage(ActivityThread.java:931)
E at android.os.Handler.dispatchMessage(Handler.java:99)
E at android.os.Looper.loop(Looper.java:130)
E at android.app.ActivityThread.main(ActivityThread.java:3683)
E at java.lang.reflect.Method.invokeNative(Native Method)
E at java.lang.reflect.Method.invoke(Method.java:507)
E at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:864)
E at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:622)
E at dalvik.system.NativeStart.main(Native Method)
```
This is because `AppSecurityPermissions` uses a method that was not added until a later API. This class is used by `AppDetails` without an API verison check around its access. Thus, we call methods on this that are unsupported when on gingerbread.
By removing `TargetApi` from the class, it unearthed a couple of locations where methods were being invoked without first guarding against the build version correctly. These two locations have been fixed and a `TargetApi` attached to the more narrowly defined method which requires it.
See merge request !353
This class is used by `AppDetails` without an API verison check around its access.
Thus, we call methods on this that are unsupported when on gingerbread.
By removing `TargetApi` from the class, it unearthed a couple of locations
where methods were being invoked without first guarding against the build version
correctly. These two locations have been fixed and a `TargetApi` attached to the
more narrowly defined method which requires it.
Added LoggingQuery for diagnostics during debug mode.
While working on #511 and investigating database performance, I found myself really
wanting to understand the bits of F-Droid which were slow and those which were fast
due to the database. I created this thing, and thought I'd create a separate MR as it is
not directly related to the other stuff, but rather a general utility class that should be helpful.
SQLite has a very nice "EXPLAIN QUERY PLAN" command (https://sqlite.org/eqp.html).
It is not really meant to be used in production code, as per the docs, but it is
super helpful at diagnosing missing indexes or other performance problems with
databases. I find it much better than, for example, the MySQL alternative.
This commit routes queries from the `ApkProvider` and `AppProvider` through a
`LoggingQuery` which (in debug builds) times queries, and if they take longer
than a certain threshold, outputs them to logcat. In addition, it will then
ask sqlite to explain its query plan for that same query, and output that to
logcat too.
I created this as a separate class because it has zero dependence on any F-Droid related
stuff and might be helpful to other people too.
See merge request !350
SQLite has a very nice "EXPLAIN QUERY PLAN" command (https://sqlite.org/eqp.html).
It is not really meant to be used in production code, as per the docs, but it is
super helpful at diagnosing missing indexes or other performance problems with
databases. I find it much better than, for example, the MySQL alternative.
This commit routes queries from the `ApkProvider` and `AppProvider` through a
`LoggingQuery` which (in debug builds) times queries, and if they take longer
than a certain threshold, outputs them to logcat. In addition, it will then
ask sqlite to explain its query plan for that same query, and output that to
logcat too.
Revert to build-tools 23 until we can have 64-bit
As long as we're stuck with 32-bit on the buildserver, avoid both target
and build-tools 24. Necessary to do an alpha.
See merge request !349
Ensure database fields referred to by `Schema.*Table.Cols.*` constants
**This is based on top of !346.** When that is merged, I'll rebase this again and then remove the WIP.
The goal of this is to ensure that all string literals which refer to database columns are replaced with constants from the relevant `Schema.*Table.Cols` interface.
The only exceptions are fields which no longer exist and are referred to in the `DBHelper` class (e.g. the `fdroid_repo` table had an `id` column but that is now `_id`).
This should not change **any** behaviour in the client app, all semantics should stay **exactly** the same.
See merge request !347
Remove LITERAL_DO from the config in RightCurly as we want this:
do {
foo;
} while (bar);
Not this:
do {
foo;
}
while (bar);
This went unnoticed as LITERAL_DO was broken in RightCurly in earlier
Checkstyle versions.
Refactor database schema constants
**Note:** When this is merged I'll rebase !347 and remove its WIP.
## Summary
In order to do the database changes required for #511, I've found that it is difficult due to my inclination to switch between referring to database columns by either a Java constant such as `AppProvider.DataColumns.NAME` and string literals such as `"name"`. All string literals should be migrated to constants, and that will happen in my next MR. In order to prepare for this, I've gathered together the constants into one common place: `org.fdroid.fdroid.data.Schema`. This is going to be the authoritative place for the schema to be stored going forward, and will help when reasoning about the database structure.
Although it seems large, this change is a fairly straightforward find/replace job. It also passes all tests for which there is good test coverage in the content providers.
## Changes
Create `Schema` interface to make it simpler to replace string literals with constants.
Right now, table names are in `DBHelper.TABLE_*` constants, and each tables fields are
in `*Provider.DataColumns.*` constants. This brings them all into a predictable location.
In addition, it makes it easier to statically import `Schema` so that instead of, e.g.,
* `AppProvider.DataColumns.PACKAGE_NAME`
We can choose one of the following, based on our current context:
* `Schema.AppTable.Cols.PACKAGE_NAME`
* `AppTable.Cols.PACKAGE_NAME`
* `Cols.PACKAGE_NAME`
In the worst case, it isa couple of chars shorter than now. In the best case, if we are
writing a class that primarily deals with Apps (e.g. App.java or AppProvider.java) then
we get a big win with just `Cols.PACKAGE_NAME`.
Having these things slightly shorter may seem like it is pointless, but the length of
each constant probably contributed to my lack of willingness to use constants instead
of string literals when constructing queries.
In the future, this should be moved towards something more akin to:
> http://openhms.sourceforge.net/sqlbuilder/
and I hope that extracting all the schema stuff into one interface may help that.
See merge request !346
Right now, table names are in `DBHelper.TABLE_*` constants, and each tables fields are
in `*Provider.DataColumns.*` constants. This brings them all into a predictable location.
In addition, it makes it easier to statically import `Schema` so that instead of, e.g.,
* `AppProvider.DataColumns.PACKAGE_NAME`
We can choose one of the following, based on our current context:
* `Schema.AppTable.Cols.PACKAGE_NAME`
* `AppTable.Cols.PACKAGE_NAME`
* `Cols.PACKAGE_NAME`
In the worst case, it isa couple of chars shorter than now. In the best case, if we are
writing a class that primarily deals with Apps (e.g. App.java or AppProvider.java) then
we get a big win with just `Cols.PACKAGE_NAME`.
Having these things slightly shorter may seem like it is pointless, but the length of
each constant probably contributed to my lack of willingness to use constants instead
of string literals when constructing queries.
In the future, this should be moved towards something more akin to:
> http://openhms.sourceforge.net/sqlbuilder/
and I hope that extracting all the schema stuff into one interface may help that.
