From 81fcd44b66a83b3889bcd75e3cd92f76d457b6f5 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 6 Mar 2014 08:05:51 +1100 Subject: [PATCH] Fixed update notification count The update notification was not taking ignored apps into account. This is the first manifestation of a class of bug I feared whereby the properties of an App object are not initialized, but no error is thrown. It occured because we were iterating over apps that were created from the index file, rather than our database. Hence, they had no knowledge about whether they should be ignored or not. Also took the chance to perform a minor refactor of UpdateService class. The onHandleIntent method was getting huge, so I extracted two methods: verifyIsTimeForScheduledRun() and performUpdateNotification(), as well as removing the unused "success" flag. The two methods should theoretically make the class more testable later, as we can test the scheduled run code, and the update notification code in separate tests, but we'll wait and see if that eventuates. --- src/org/fdroid/fdroid/UpdateService.java | 147 +++++++++++--------- src/org/fdroid/fdroid/data/AppProvider.java | 28 +++- 2 files changed, 110 insertions(+), 65 deletions(-) diff --git a/src/org/fdroid/fdroid/UpdateService.java b/src/org/fdroid/fdroid/UpdateService.java index 5ff446905..f22b93281 100644 --- a/src/org/fdroid/fdroid/UpdateService.java +++ b/src/org/fdroid/fdroid/UpdateService.java @@ -201,6 +201,45 @@ public class UpdateService extends IntentService implements ProgressListener { return receiver == null; } + /** + * Check whether it is time to run the scheduled update. + * We don't want to run if: + * - The time between scheduled runs is set to zero (though don't know + * when that would occur) + * - Last update was too recent + * - Not on wifi, but the property for "Only auto update on wifi" is set. + * @return True if we are due for a scheduled update. + */ + private boolean verifyIsTimeForScheduledRun() { + SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(getBaseContext()); + long lastUpdate = prefs.getLong(Preferences.PREF_UPD_LAST, 0); + String sint = prefs.getString(Preferences.PREF_UPD_INTERVAL, "0"); + int interval = Integer.parseInt(sint); + if (interval == 0) { + Log.d("FDroid", "Skipping update - disabled"); + return false; + } + long elapsed = System.currentTimeMillis() - lastUpdate; + if (elapsed < interval * 60 * 60 * 1000) { + Log.d("FDroid", "Skipping update - done " + elapsed + + "ms ago, interval is " + interval + " hours"); + return false; + } + + // If we are to update the repos only on wifi, make sure that + // connection is active + if (prefs.getBoolean(Preferences.PREF_UPD_WIFI_ONLY, false)) { + ConnectivityManager conMan = (ConnectivityManager) getSystemService(Context.CONNECTIVITY_SERVICE); + NetworkInfo.State wifi = conMan.getNetworkInfo(1).getState(); + if (wifi != NetworkInfo.State.CONNECTED && + wifi != NetworkInfo.State.CONNECTING) { + Log.d("FDroid", "Skipping update - wifi not available"); + return false; + } + } + return true; + } + @Override protected void onHandleIntent(Intent intent) { @@ -210,39 +249,13 @@ public class UpdateService extends IntentService implements ProgressListener { long startTime = System.currentTimeMillis(); String errmsg = ""; try { - - SharedPreferences prefs = PreferenceManager - .getDefaultSharedPreferences(getBaseContext()); + SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(getBaseContext()); // See if it's time to actually do anything yet... - if (isScheduledRun()) { - long lastUpdate = prefs.getLong(Preferences.PREF_UPD_LAST, 0); - String sint = prefs.getString(Preferences.PREF_UPD_INTERVAL, "0"); - int interval = Integer.parseInt(sint); - if (interval == 0) { - Log.d("FDroid", "Skipping update - disabled"); - return; - } - long elapsed = System.currentTimeMillis() - lastUpdate; - if (elapsed < interval * 60 * 60 * 1000) { - Log.d("FDroid", "Skipping update - done " + elapsed - + "ms ago, interval is " + interval + " hours"); - return; - } - - // If we are to update the repos only on wifi, make sure that - // connection is active - if (prefs.getBoolean(Preferences.PREF_UPD_WIFI_ONLY, false)) { - ConnectivityManager conMan = (ConnectivityManager) getSystemService(Context.CONNECTIVITY_SERVICE); - NetworkInfo.State wifi = conMan.getNetworkInfo(1).getState(); - if (wifi != NetworkInfo.State.CONNECTED && - wifi != NetworkInfo.State.CONNECTING) { - Log.d("FDroid", "Skipping update - wifi not available"); - return; - } - } - } else { + if (!isScheduledRun()) { Log.d("FDroid", "Unscheduled (manually requested) update"); + } else if (!verifyIsTimeForScheduledRun()) { + return; } // Grab some preliminary information, then we can release the @@ -255,7 +268,6 @@ public class UpdateService extends IntentService implements ProgressListener { List unchangedRepos = new ArrayList(); List updatedRepos = new ArrayList(); List disabledRepos = new ArrayList(); - boolean success = true; boolean changes = false; for (Repo repo : repos) { @@ -289,14 +301,10 @@ public class UpdateService extends IntentService implements ProgressListener { } } - if (!changes && success) { - Log.d("FDroid", - "Not checking app details or compatibility, " + - "because all repos were up to date."); - } else if (changes && success) { - - sendStatus(STATUS_INFO, - getString(R.string.status_checking_compatibility)); + if (!changes) { + Log.d("FDroid", "Not checking app details or compatibility, ecause all repos were up to date."); + } else { + sendStatus(STATUS_INFO, getString(R.string.status_checking_compatibility)); List listOfAppsToUpdate = new ArrayList(); listOfAppsToUpdate.addAll(appsToUpdate.values()); @@ -312,34 +320,19 @@ public class UpdateService extends IntentService implements ProgressListener { removeApksNoLongerInRepo(listOfAppsToUpdate, updatedRepos); removeAppsWithoutApks(); notifyContentProviders(); - } - if (success && changes && prefs.getBoolean(Preferences.PREF_UPD_NOTIFY, false)) { - int updateCount = 0; - for (App app : appsToUpdate.values()) { - if (app.canAndWantToUpdate(this)) { - updateCount++; - } - } - - if (updateCount > 0) { - showAppUpdatesNotification(updateCount); + if (prefs.getBoolean(Preferences.PREF_UPD_NOTIFY, false)) { + performUpdateNotification(appsToUpdate.values()); } } - if (!success) { - if (errmsg.length() == 0) - errmsg = "Unknown error"; - sendStatus(STATUS_ERROR, errmsg); + Editor e = prefs.edit(); + e.putLong(Preferences.PREF_UPD_LAST, System.currentTimeMillis()); + e.commit(); + if (changes) { + sendStatus(STATUS_COMPLETE_WITH_CHANGES); } else { - Editor e = prefs.edit(); - e.putLong(Preferences.PREF_UPD_LAST, System.currentTimeMillis()); - e.commit(); - if (changes) { - sendStatus(STATUS_COMPLETE_WITH_CHANGES); - } else { - sendStatus(STATUS_COMPLETE_AND_SAME); - } + sendStatus(STATUS_COMPLETE_AND_SAME); } } catch (Exception e) { @@ -464,7 +457,35 @@ public class UpdateService extends IntentService implements ProgressListener { } } - private void showAppUpdatesNotification(int updates) throws Exception { + private void performUpdateNotification(Collection apps) { + int updateCount = 0; + + // This may be somewhat strange, because we usually would just trust + // App.canAndWantToUpdate(). The only problem is that the "appsToUpdate" + // list only contains data from the repo index, not our database. + // As such, it doesn't know if we want to ignore the apps or not. For that, we + // need to query the database manually and identify those which are to be ignored. + String[] projection = { AppProvider.DataColumns.APP_ID }; + List appsToIgnore = AppProvider.Helper.findIgnored(this, projection); + for (App app : apps) { + boolean ignored = false; + for(App appIgnored : appsToIgnore) { + if (appIgnored.id.equals(app.id)) { + ignored = true; + break; + } + } + if (!ignored && app.hasUpdates(this)) { + updateCount++; + } + } + + if (updateCount > 0) { + showAppUpdatesNotification(updateCount); + } + } + + private void showAppUpdatesNotification(int updates) { Log.d("FDroid", "Notifying " + updates + " updates."); NotificationCompat.Builder builder = new NotificationCompat.Builder(this) diff --git a/src/org/fdroid/fdroid/data/AppProvider.java b/src/org/fdroid/fdroid/data/AppProvider.java index 9475cfb62..76d0121fb 100644 --- a/src/org/fdroid/fdroid/data/AppProvider.java +++ b/src/org/fdroid/fdroid/data/AppProvider.java @@ -7,6 +7,7 @@ import android.net.Uri; import android.util.Log; import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.R; +import org.fdroid.fdroid.UpdateService; import org.fdroid.fdroid.Utils; import java.util.*; @@ -32,6 +33,12 @@ public class AppProvider extends FDroidProvider { return cursorToList(cursor); } + public static List findIgnored(Context context, String[] projection) { + Uri uri = AppProvider.getIgnoredUri(); + Cursor cursor = context.getContentResolver().query(uri, projection, null, null, null); + return cursorToList(cursor); + } + private static List cursorToList(Cursor cursor) { List apps = new ArrayList(); if (cursor != null) { @@ -227,6 +234,7 @@ public class AppProvider extends FDroidProvider { private static final String PATH_RECENTLY_UPDATED = "recentlyUpdated"; private static final String PATH_NEWLY_ADDED = "newlyAdded"; private static final String PATH_CATEGORY = "category"; + private static final String PATH_IGNORED = "ignored"; private static final int CAN_UPDATE = CODE_SINGLE + 1; private static final int INSTALLED = CAN_UPDATE + 1; @@ -236,9 +244,11 @@ public class AppProvider extends FDroidProvider { private static final int RECENTLY_UPDATED = APPS + 1; private static final int NEWLY_ADDED = RECENTLY_UPDATED + 1; private static final int CATEGORY = NEWLY_ADDED + 1; + private static final int IGNORED = CATEGORY + 1; static { matcher.addURI(getAuthority(), null, CODE_LIST); + matcher.addURI(getAuthority(), PATH_IGNORED, IGNORED); matcher.addURI(getAuthority(), PATH_RECENTLY_UPDATED, RECENTLY_UPDATED); matcher.addURI(getAuthority(), PATH_NEWLY_ADDED, NEWLY_ADDED); matcher.addURI(getAuthority(), PATH_CATEGORY + "/*", CATEGORY); @@ -262,6 +272,10 @@ public class AppProvider extends FDroidProvider { return Uri.withAppendedPath(getContentUri(), PATH_NEWLY_ADDED); } + public static Uri getIgnoredUri() { + return Uri.withAppendedPath(getContentUri(), PATH_IGNORED); + } + public static Uri getCategoryUri(String category) { return getContentUri().buildUpon() .appendPath(PATH_CATEGORY) @@ -388,6 +402,12 @@ public class AppProvider extends FDroidProvider { return new QuerySelection(selection, args); } + private QuerySelection queryIgnored() { + String selection = "fdroid_app.ignoreAllUpdates = 1 OR " + + "fdroid_app.ignoreThisUpdate >= fdroid_app.suggestedVercode"; + return new QuerySelection(selection); + } + private QuerySelection queryNewlyAdded() { String selection = "fdroid_app.added > ?"; String[] args = { Utils.DATE_FORMAT.format(Preferences.get().calcMaxHistory()) }; @@ -459,6 +479,10 @@ public class AppProvider extends FDroidProvider { query = query.add(queryApps(uri.getLastPathSegment())); break; + case IGNORED: + query = query.add(queryIgnored()); + break; + case CATEGORY: query = query.add(queryCategory(uri.getLastPathSegment())); break; @@ -503,7 +527,7 @@ public class AppProvider extends FDroidProvider { break; default: - throw new UnsupportedOperationException("Can't delete yet"); + throw new UnsupportedOperationException("Delete not supported for " + uri + "."); } @@ -531,7 +555,7 @@ public class AppProvider extends FDroidProvider { break; default: - throw new UnsupportedOperationException("Update not supported for '" + uri + "'."); + throw new UnsupportedOperationException("Update not supported for " + uri + "."); } int count = write().update(getTableName(), values, query.getSelection(), query.getArgs());