Merge branch 'ensure-large-icon-urls-exist' of https://gitlab.com/pserwylo/fdroidclient

F-Droid/src/org/fdroid/fdroid/Utils.java

@@ -432,7 +432,8 @@ public final class Utils {
             return new CommaSeparatedList(sb.toString());
         }
 
-        public static CommaSeparatedList make(String list) {
+        @Nullable
+        public static CommaSeparatedList make(@Nullable String list) {
             if (TextUtils.isEmpty(list))
                 return null;
             return new CommaSeparatedList(list);

F-Droid/src/org/fdroid/fdroid/data/AppProvider.java

@@ -5,6 +5,7 @@ import android.content.ContentValues;
 import android.content.Context;
 import android.content.UriMatcher;
 import android.database.Cursor;
+import android.database.sqlite.SQLiteDatabase;
 import android.net.Uri;
 import android.util.Log;
 
@@ -98,8 +99,9 @@ public class AppProvider extends FDroidProvider {
                     cursor.moveToFirst();
                     while (!cursor.isAfterLast()) {
                         final String categoriesString = cursor.getString(0);
-                        if (categoriesString != null) {
+                        Utils.CommaSeparatedList categoriesList = Utils.CommaSeparatedList.make(categoriesString);
-                            for (final String s : Utils.CommaSeparatedList.make(categoriesString)) {
+                        if (categoriesList != null) {
+                            for (final String s : categoriesList) {
                                 categorySet.add(s);
                             }
                         }
@@ -152,6 +154,22 @@ public class AppProvider extends FDroidProvider {
 
     }
 
+    /**
+     * Class that only exists to call private methods in the {@link AppProvider} without having
+     * to go via a Context/ContentResolver. The reason is that if the {@link DBHelper} class
+     * was to try and use its getContext().getContentResolver() in order to access the app
+     * provider, then the AppProvider will end up creating a new instance of a writeable
+     * SQLiteDatabase. This causes problems because the {@link DBHelper} still has its reference
+     * open and locks certain tables.
+     */
+    static final class UpgradeHelper {
+
+        public static void updateIconUrls(Context context, SQLiteDatabase db) {
+            AppProvider.updateIconUrls(context, db);
+        }
+
+    }
+
     public interface DataColumns {
 
         String _ID = "rowid as _id"; // Required for CursorLoaders
@@ -790,7 +808,7 @@ public class AppProvider extends FDroidProvider {
         updateCompatibleFlags();
         updateSuggestedFromLatest();
         updateSuggestedFromUpstream();
-        updateIconUrls();
+        updateIconUrls(getContext(), write());
     }
 
     /**
@@ -923,11 +941,13 @@ public class AppProvider extends FDroidProvider {
     }
 
     /**
-     * Updates URLs to icons
+     * Made static so that the {@link org.fdroid.fdroid.data.AppProvider.UpgradeHelper} can access
+     * it without instantiating an {@link AppProvider}. This is also the reason it needs to accept
+     * the context and database as arguments.
      */
-    public void updateIconUrls() {
+    private static void updateIconUrls(Context context, SQLiteDatabase db) {
-        final String iconsDir = Utils.getIconsDir(getContext(), 1.0);
+        final String iconsDir = Utils.getIconsDir(context, 1.0);
-        final String iconsDirLarge = Utils.getIconsDir(getContext(), 1.5);
+        final String iconsDirLarge = Utils.getIconsDir(context, 1.5);
         String repoVersion = Integer.toString(Repo.VERSION_DENSITY_SPECIFIC_ICONS);
         Log.d(TAG, "Updating icon paths for apps belonging to repos with version >= "
                 + repoVersion);
@@ -937,7 +957,7 @@ public class AppProvider extends FDroidProvider {
         final String[] params = {
             repoVersion, iconsDir, Utils.FALLBACK_ICONS_DIR,
             repoVersion, iconsDirLarge, Utils.FALLBACK_ICONS_DIR };
-        write().execSQL(query, params);
+        db.execSQL(query, params);
     }
 
     /**
@@ -945,7 +965,7 @@ public class AppProvider extends FDroidProvider {
      *  1) The repo version that introduced density specific icons
      *  2) The dir to density specific icons for the current device.
      */
-    private String getIconUpdateQuery() {
+    private static String getIconUpdateQuery() {
 
         final String apk = DBHelper.TABLE_APK;
         final String app = DBHelper.TABLE_APP;

F-Droid/src/org/fdroid/fdroid/data/DBHelper.java

@@ -101,7 +101,7 @@ public class DBHelper extends SQLiteOpenHelper {
             + InstalledAppProvider.DataColumns.APPLICATION_LABEL + " TEXT NOT NULL "
             + " );";
 
-    private static final int DB_VERSION = 49;
+    private static final int DB_VERSION = 50;
 
     private final Context context;
 
@@ -284,6 +284,7 @@ public class DBHelper extends SQLiteOpenHelper {
         addIsSwapToRepo(db, oldVersion);
         addChangelogToApp(db, oldVersion);
         addIconUrlLargeToApp(db, oldVersion);
+        updateIconUrlLarge(db, oldVersion);
     }
 
     /**
@@ -432,6 +433,24 @@ public class DBHelper extends SQLiteOpenHelper {
         }
     }
 
+    private void updateIconUrlLarge(SQLiteDatabase db, int oldVersion) {
+        if (oldVersion < 50) {
+            Log.i(TAG, "Recalculating app icon URLs so that the newly added large icons will get updated.");
+            AppProvider.UpgradeHelper.updateIconUrls(context, db);
+            clearRepoEtags(db);
+        }
+    }
+
+    /**
+     * By clearing the etags stored in the repo table, it means that next time the user updates
+     * their repos (either manually or on a scheduled task), they will update regardless of whether
+     * they have changed since last update or not.
+     */
+    private void clearRepoEtags(SQLiteDatabase db) {
+        Log.i(TAG, "Clearing repo etags, so next update will not be skipped with \"Repos up to date\".");
+        db.execSQL("update " + TABLE_REPO + " set lastetag = NULL");
+    }
+
     private void resetTransient(SQLiteDatabase db, int oldVersion) {
         // Before version 42, only transient info was stored in here. As of some time
         // just before 42 (F-Droid 0.60ish) it now has "ignore this version" info which
@@ -443,7 +462,7 @@ public class DBHelper extends SQLiteOpenHelper {
                     .putBoolean("triedEmptyUpdate", false).commit();
             db.execSQL("drop table " + TABLE_APP);
             db.execSQL("drop table " + TABLE_APK);
-            db.execSQL("update " + TABLE_REPO + " set lastetag = NULL");
+            clearRepoEtags(db);
             createAppApk(db);
         }
     }