Merge branch 'issue-1143--performance' into 'master'

performance refactor for isApp --> isApk

Closes #1143 and #1156

See merge request !579

app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java

@@ -226,6 +226,12 @@ public class IndexV1Updater extends RepoUpdater {
 
                 if (apks.size() > 0) {
                     app.preferredSigner = apks.get(0).sig;
+                    app.isApk = true;
+                    for (Apk apk : apks) {
+                        if (!apk.isApk()) {
+                            app.isApk = false;
+                        }
+                    }
                 }
 
                 if (appCount % 50 == 0) {

app/src/main/java/org/fdroid/fdroid/data/App.java

@@ -104,6 +104,8 @@ public class App extends ValueObject implements Comparable<App>, Parcelable {
     @JsonIgnore
     @NonNull
     public String preferredSigner;
+    @JsonIgnore
+    public boolean isApk;
 
     @JacksonInject("repoId")
     public long repoId;
@@ -347,6 +349,9 @@ public class App extends ValueObject implements Comparable<App>, Parcelable {
                 case Cols.WEAR_SCREENSHOTS:
                     wearScreenshots = Utils.parseCommaSeparatedString(cursor.getString(i));
                     break;
+                case Cols.IS_APK:
+                    isApk = cursor.getInt(i) == 1;
+                    break;
                 case Cols.InstalledApp.VERSION_CODE:
                     installedVersionCode = cursor.getInt(i);
                     break;
@@ -854,12 +859,19 @@ public class App extends ValueObject implements Comparable<App>, Parcelable {
         values.put(Cols.TV_SCREENSHOTS, Utils.serializeCommaSeparatedString(tvScreenshots));
         values.put(Cols.WEAR_SCREENSHOTS, Utils.serializeCommaSeparatedString(wearScreenshots));
         values.put(Cols.IS_COMPATIBLE, compatible ? 1 : 0);
+        values.put(Cols.IS_APK, isApk ? 1 : 0);
 
         return values;
     }
 
     public boolean isInstalled(Context context) {
-        return installedVersionCode > 0 || isMediaInstalled(context);
+        // First check isApk() before isMediaInstalled() because the latter is quite expensive,
+        // hitting the database for each apk version, then the disk to check for installed media.
+        return installedVersionCode > 0 || (!isApk() && isMediaInstalled(context));
+    }
+
+    private boolean isApk() {
+        return isApk;
     }
 
     public boolean isMediaInstalled(Context context) {
@@ -1064,6 +1076,7 @@ public class App extends ValueObject implements Comparable<App>, Parcelable {
         dest.writeStringArray(this.tenInchScreenshots);
         dest.writeStringArray(this.tvScreenshots);
         dest.writeStringArray(this.wearScreenshots);
+        dest.writeByte(this.isApk ? (byte) 1 : (byte) 0);
         dest.writeString(this.installedVersionName);
         dest.writeInt(this.installedVersionCode);
         dest.writeParcelable(this.installedApk, flags);
@@ -1114,6 +1127,7 @@ public class App extends ValueObject implements Comparable<App>, Parcelable {
         this.tenInchScreenshots = in.createStringArray();
         this.tvScreenshots = in.createStringArray();
         this.wearScreenshots = in.createStringArray();
+        this.isApk = in.readByte() != 0;
         this.installedVersionName = in.readString();
         this.installedVersionCode = in.readInt();
         this.installedApk = in.readParcelable(Apk.class.getClassLoader());
@@ -1140,14 +1154,18 @@ public class App extends ValueObject implements Comparable<App>, Parcelable {
      * However, if the app is installed, then we override this and instead want to only encourage
      * the user to try and install versions with that signature (because thats all the OS will let
      * them do).
+     *
+     * Will return null for any {@link App} which represents media (instead of an apk) and thus
+     * doesn't have a signer.
      */
-    @NonNull
+    @Nullable
     public String getMostAppropriateSignature() {
         if (!TextUtils.isEmpty(installedSig)) {
             return installedSig;
         } else if (!TextUtils.isEmpty(preferredSigner)) {
             return preferredSigner;
         }
-        throw new IllegalStateException("Most Appropriate Signature not found!");
+
+        return null;
     }
 }

app/src/main/java/org/fdroid/fdroid/data/AppProvider.java

@@ -217,7 +217,6 @@ public class AppProvider extends FDroidProvider {
         protected String getRequiredTables() {
             final String pkg  = PackageTable.NAME;
             final String app  = getTableName();
-            final String apk  = getApkTableName();
             final String repo = RepoTable.NAME;
             final String cat  = CategoryTable.NAME;
             final String catJoin = getCatJoinTableName();
@@ -226,8 +225,7 @@ public class AppProvider extends FDroidProvider {
                 " JOIN " + app + " ON (" + app + "." + Cols.PACKAGE_ID + " = " + pkg + "." + PackageTable.Cols.ROW_ID + ") " +
                 " JOIN " + repo + " ON (" + app + "." + Cols.REPO_ID + " = " + repo + "." + RepoTable.Cols._ID + ") " +
                 " LEFT JOIN " + catJoin + " ON (" + app + "." + Cols.ROW_ID + " = " + catJoin + "." + CatJoinTable.Cols.APP_METADATA_ID + ") " +
-                " LEFT JOIN " + cat + " ON (" + cat + "." + CategoryTable.Cols.ROW_ID + " = " + catJoin + "." + CatJoinTable.Cols.CATEGORY_ID + ") " +
+                " LEFT JOIN " + cat + " ON (" + cat + "." + CategoryTable.Cols.ROW_ID + " = " + catJoin + "." + CatJoinTable.Cols.CATEGORY_ID + ") ";
-                " LEFT JOIN " + apk + " ON (" + apk + "." + ApkTable.Cols.APP_ID + " = " + app + "." + Cols.ROW_ID + ") ";
         }
 
         @Override

app/src/main/java/org/fdroid/fdroid/data/DBHelper.java

@@ -151,6 +151,7 @@ class DBHelper extends SQLiteOpenHelper {
             + AppMetadataTable.Cols.TEN_INCH_SCREENSHOTS + " string,"
             + AppMetadataTable.Cols.TV_SCREENSHOTS + " string,"
             + AppMetadataTable.Cols.WEAR_SCREENSHOTS + " string,"
+            + AppMetadataTable.Cols.IS_APK + " boolean,"
             + "primary key(" + AppMetadataTable.Cols.PACKAGE_ID + ", " + AppMetadataTable.Cols.REPO_ID + "));";
 
     private static final String CREATE_TABLE_APP_PREFS = "CREATE TABLE " + AppPrefsTable.NAME
@@ -193,7 +194,7 @@ class DBHelper extends SQLiteOpenHelper {
             + InstalledAppTable.Cols.HASH + " TEXT NOT NULL"
             + " );";
 
-    protected static final int DB_VERSION = 73;
+    protected static final int DB_VERSION = 74;
 
     private final Context context;
 
@@ -281,6 +282,30 @@ class DBHelper extends SQLiteOpenHelper {
         addIntegerPrimaryKeyToInstalledApps(db, oldVersion);
         addPreferredSignerToApp(db, oldVersion);
         updatePreferredSignerIfEmpty(db, oldVersion);
+        addIsAppToApp(db, oldVersion);
+    }
+
+    private void addIsAppToApp(SQLiteDatabase db, int oldVersion) {
+        if (oldVersion >= 74) {
+            return;
+        }
+
+        if (!columnExists(db, AppMetadataTable.NAME, AppMetadataTable.Cols.IS_APK)) {
+            Log.i(TAG, "Figuring out whether each \"app\" is actually an app, or it represents other media.");
+            db.execSQL("alter table " + AppMetadataTable.NAME + " add column " + AppMetadataTable.Cols.IS_APK + " boolean;");
+
+            // Find all apks for which their filename DOESN'T end in ".apk", and if there is more than one, the
+            // corresponding app is updated to be marked as media.
+            String apkName = ApkTable.Cols.NAME;
+            String query = "UPDATE " + AppMetadataTable.NAME + " SET " + AppMetadataTable.Cols.IS_APK + " = (" +
+                    "  SELECT COUNT(*) FROM " + ApkTable.NAME + " AS apk" +
+                    "  WHERE " +
+                    "    " + ApkTable.Cols.APP_ID + " = " + AppMetadataTable.NAME + "." + AppMetadataTable.Cols.ROW_ID +
+                    "    AND SUBSTR(" + apkName + ", LENGTH(" + apkName + ") - 3) != '.apk'" +
+                    ") = 0;";
+            Log.i(TAG, query);
+            db.execSQL(query);
+        }
     }
 
     private void updatePreferredSignerIfEmpty(SQLiteDatabase db, int oldVersion) {

app/src/main/java/org/fdroid/fdroid/data/LoggingQuery.java

@@ -1,6 +1,7 @@
 package org.fdroid.fdroid.data;
 
 import android.database.Cursor;
+import android.database.CursorWrapper;
 import android.database.sqlite.SQLiteDatabase;
 
 import org.fdroid.fdroid.BuildConfig;
@@ -61,11 +62,37 @@ final class LoggingQuery {
             if (queryDuration >= SLOW_QUERY_DURATION) {
                 logSlowQuery(queryDuration);
             }
-            return cursor;
+
+            return new LogGetCountCursorWrapper(cursor);
         }
         return db.rawQuery(query, queryArgs);
     }
 
+    /**
+     * Sometimes the query will not actually be run when invoking "query()".
+     * Under such circumstances, it falls to the {@link android.content.ContentProvider#query}
+     * method to manually invoke the {@link Cursor#getCount()} method to force query execution.
+     * It does so with a comment saying "Force query execution". When this happens, the call to
+     * query() takes 1ms, whereas the call go getCount() is the bit which takes time.
+     * As such, we will also track that method duration in order to potentially log slow queries.
+     */
+    private final class LogGetCountCursorWrapper extends CursorWrapper {
+        private LogGetCountCursorWrapper(Cursor cursor) {
+            super(cursor);
+        }
+
+        @Override
+        public int getCount() {
+            long startTime = System.currentTimeMillis();
+            int count = super.getCount();
+            long queryDuration = System.currentTimeMillis() - startTime;
+            if (queryDuration >= SLOW_QUERY_DURATION) {
+                logSlowQuery(queryDuration);
+            }
+            return count;
+        }
+    }
+
     private void execSQLInternal() {
         if (BuildConfig.DEBUG) {
             long startTime = System.currentTimeMillis();

app/src/main/java/org/fdroid/fdroid/data/Schema.java

@@ -156,6 +156,7 @@ public interface Schema {
             String TEN_INCH_SCREENSHOTS = "tenInchScreenshots";
             String TV_SCREENSHOTS = "tvScreenshots";
             String WEAR_SCREENSHOTS = "wearScreenshots";
+            String IS_APK = "isApk";
 
             interface SuggestedApk {
                 String VERSION_NAME = "suggestedApkVersion";
@@ -195,7 +196,7 @@ public interface Schema {
                     ANTI_FEATURES, REQUIREMENTS, ICON_URL, ICON_URL_LARGE,
                     FEATURE_GRAPHIC, PROMO_GRAPHIC, TV_BANNER, PHONE_SCREENSHOTS,
                     SEVEN_INCH_SCREENSHOTS, TEN_INCH_SCREENSHOTS, TV_SCREENSHOTS, WEAR_SCREENSHOTS,
-                    PREFERRED_SIGNER, SUGGESTED_VERSION_CODE,
+                    PREFERRED_SIGNER, SUGGESTED_VERSION_CODE, IS_APK,
             };
 
             /**
@@ -211,7 +212,7 @@ public interface Schema {
                     ANTI_FEATURES, REQUIREMENTS, ICON_URL, ICON_URL_LARGE,
                     FEATURE_GRAPHIC, PROMO_GRAPHIC, TV_BANNER, PHONE_SCREENSHOTS,
                     SEVEN_INCH_SCREENSHOTS, TEN_INCH_SCREENSHOTS, TV_SCREENSHOTS, WEAR_SCREENSHOTS,
-                    PREFERRED_SIGNER, SUGGESTED_VERSION_CODE, SuggestedApk.VERSION_NAME,
+                    PREFERRED_SIGNER, SUGGESTED_VERSION_CODE, IS_APK, SuggestedApk.VERSION_NAME,
                     InstalledApp.VERSION_CODE, InstalledApp.VERSION_NAME,
                     InstalledApp.SIGNATURE, Package.PACKAGE_NAME,
             };

app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java

@@ -304,6 +304,7 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
                 "installedSig",
                 "installedVersionCode",
                 "installedVersionName",
+                "isApk",
                 "preferredSigner",
                 "prefs",
                 "TAG",
@@ -335,6 +336,7 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
                 "hash",
                 "hashType",
                 "incompatibleReasons",
+                "isApk",
                 "maxSdkVersion",
                 "minSdkVersion",
                 "nativecode",