From e1a8c42f9e3df1f988fa334895a55a7ec86b404f Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 30 Jul 2015 17:05:16 +1000 Subject: [PATCH 1/2] Ensure icon URLs are always populated when database is upgraded. Did this by using the same query which is used to update the icon URLs after updating the index. To make this same method accessible, without causing sad database locking, had to expose the method from AppProvider in a way which would let DBHelper access it. See comments in code for further explainations. While there, removed the final lint warning in my Android Studio for the AppProvider. This was a warning because we could have ended up iterating over a null object. Although it turns out there was a correct guard in place which would ensure this didn't happen, it wasn't in such a way that lint would understand. Thus, I changed the guard condition around `for( String blah : CommaSeparatedList.make() ) {}` to let lint relax and not be so pedantic. --- F-Droid/src/org/fdroid/fdroid/Utils.java | 3 +- .../org/fdroid/fdroid/data/AppProvider.java | 38 ++++++++++++++----- .../src/org/fdroid/fdroid/data/DBHelper.java | 10 ++++- 3 files changed, 40 insertions(+), 11 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/Utils.java b/F-Droid/src/org/fdroid/fdroid/Utils.java index cce9d61cd..3e2836ce2 100644 --- a/F-Droid/src/org/fdroid/fdroid/Utils.java +++ b/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); diff --git a/F-Droid/src/org/fdroid/fdroid/data/AppProvider.java b/F-Droid/src/org/fdroid/fdroid/data/AppProvider.java index 7c223e3b5..78d2792b0 100644 --- a/F-Droid/src/org/fdroid/fdroid/data/AppProvider.java +++ b/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) { - for (final String s : Utils.CommaSeparatedList.make(categoriesString)) { + Utils.CommaSeparatedList categoriesList = 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() { - final String iconsDir = Utils.getIconsDir(getContext(), 1.0); - final String iconsDirLarge = Utils.getIconsDir(getContext(), 1.5); + private static void updateIconUrls(Context context, SQLiteDatabase db) { + final String iconsDir = Utils.getIconsDir(context, 1.0); + 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; diff --git a/F-Droid/src/org/fdroid/fdroid/data/DBHelper.java b/F-Droid/src/org/fdroid/fdroid/data/DBHelper.java index 86b8f363f..98f1d39a0 100644 --- a/F-Droid/src/org/fdroid/fdroid/data/DBHelper.java +++ b/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,13 @@ 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); + } + } + 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 From 4565f58a75aa911fd3e860ac8aa6168fa3a39951 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 30 Jul 2015 17:30:58 +1000 Subject: [PATCH 2/2] Clear out the etag from the repo table to "force" update. We are not forcing an update, in the sense that we make the update service run. Rather, we are ensuring that the next update wont return after doing nothing, with the message "repos already up to date". In this case, the repo metadata (and hence its etag) is the same, but we made changes in the client to handle the metadata correctly. Thus, we don't care that it hasn't changed, we want to update anyhow. --- F-Droid/src/org/fdroid/fdroid/data/DBHelper.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/F-Droid/src/org/fdroid/fdroid/data/DBHelper.java b/F-Droid/src/org/fdroid/fdroid/data/DBHelper.java index 98f1d39a0..ea3a153b0 100644 --- a/F-Droid/src/org/fdroid/fdroid/data/DBHelper.java +++ b/F-Droid/src/org/fdroid/fdroid/data/DBHelper.java @@ -437,9 +437,20 @@ public class DBHelper extends SQLiteOpenHelper { 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 @@ -451,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); } }