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

@@ -201,33 +201,29 @@ public class UpdateService extends IntentService implements ProgressListener {
         return receiver == null;
     }
 
-    @Override
+    /**
-    protected void onHandleIntent(Intent intent) {
+     * Check whether it is time to run the scheduled update.
-
+     * We don't want to run if:
-        receiver = intent.getParcelableExtra("receiver");
+     *  - The time between scheduled runs is set to zero (though don't know
-        String address = intent.getStringExtra("address");
+     *    when that would occur)
-
+     *  - Last update was too recent
-        long startTime = System.currentTimeMillis();
+     *  - Not on wifi, but the property for "Only auto update on wifi" is set.
-        String errmsg = "";
+     * @return True if we are due for a scheduled update.
-        try {
+     */
-
+    private boolean verifyIsTimeForScheduledRun() {
-            SharedPreferences prefs = PreferenceManager
+        SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(getBaseContext());
-                    .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;
+            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;
+            return false;
         }
 
         // If we are to update the repos only on wifi, make sure that
@@ -238,11 +234,28 @@ public class UpdateService extends IntentService implements ProgressListener {
             if (wifi != NetworkInfo.State.CONNECTED &&
                     wifi !=  NetworkInfo.State.CONNECTING) {
                 Log.d("FDroid", "Skipping update - wifi not available");
-                        return;
+                return false;
             }
         }
-            } else {
+        return true;
+    }
+
+    @Override
+    protected void onHandleIntent(Intent intent) {
+
+        receiver = intent.getParcelableExtra("receiver");
+        String address = intent.getStringExtra("address");
+
+        long startTime = System.currentTimeMillis();
+        String errmsg = "";
+        try {
+            SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(getBaseContext());
+
+            // See if it's time to actually do anything yet...
+            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<Repo> unchangedRepos = new ArrayList<Repo>();
             List<Repo> updatedRepos = new ArrayList<Repo>();
             List<Repo> disabledRepos = new ArrayList<Repo>();
-            boolean success = true;
             boolean changes = false;
             for (Repo repo : repos) {
 
@@ -289,14 +301,10 @@ public class UpdateService extends IntentService implements ProgressListener {
                 }
             }
 
-            if (!changes && success) {
+            if (!changes) {
-                Log.d("FDroid",
+                Log.d("FDroid", "Not checking app details or compatibility, ecause all repos were up to date.");
-                        "Not checking app details or compatibility, " +
+            } else {
-                                "because all repos were up to date.");
+                sendStatus(STATUS_INFO, getString(R.string.status_checking_compatibility));
-            } else if (changes && success) {
-
-                sendStatus(STATUS_INFO,
-                        getString(R.string.status_checking_compatibility));
 
                 List<App> listOfAppsToUpdate = new ArrayList<App>();
                 listOfAppsToUpdate.addAll(appsToUpdate.values());
@@ -312,26 +320,12 @@ public class UpdateService extends IntentService implements ProgressListener {
                 removeApksNoLongerInRepo(listOfAppsToUpdate, updatedRepos);
                 removeAppsWithoutApks();
                 notifyContentProviders();
-            }
 
-            if (success && changes && prefs.getBoolean(Preferences.PREF_UPD_NOTIFY, false)) {
+                if (prefs.getBoolean(Preferences.PREF_UPD_NOTIFY, false)) {
-                int updateCount = 0;
+                    performUpdateNotification(appsToUpdate.values());
-                for (App app : appsToUpdate.values()) {
-                    if (app.canAndWantToUpdate(this)) {
-                        updateCount++;
                 }
             }
 
-                if (updateCount > 0) {
-                    showAppUpdatesNotification(updateCount);
-                }
-            }
-
-            if (!success) {
-                if (errmsg.length() == 0)
-                    errmsg = "Unknown error";
-                sendStatus(STATUS_ERROR, errmsg);
-            } else {
             Editor e = prefs.edit();
             e.putLong(Preferences.PREF_UPD_LAST, System.currentTimeMillis());
             e.commit();
@@ -340,7 +334,6 @@ public class UpdateService extends IntentService implements ProgressListener {
             } else {
                 sendStatus(STATUS_COMPLETE_AND_SAME);
             }
-            }
 
         } catch (Exception e) {
             Log.e("FDroid",
@@ -464,7 +457,35 @@ public class UpdateService extends IntentService implements ProgressListener {
         }
     }
 
-    private void showAppUpdatesNotification(int updates) throws Exception {
+    private void performUpdateNotification(Collection<App> 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<App> 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)

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<App> 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<App> cursorToList(Cursor cursor) {
             List<App> apps = new ArrayList<App>();
             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());