Add progress reporting for index-v1.

Reuses the "commiting" message to indicate how many apps have been
processed so far.

Refactors existing progress handling between `RepoUpdater` and
`UpdateService` to use `LocalBroadcastManager` in preference to
`ProgressListener`. Still needs to use `ProgressListener` to talk
between `RepoUpdater` and the `Downloader` +
`ProgressBufferedInputStream`.

The only change that is related to something more important than
notifications is the fact that now `IndexV1Updater` makes use of the
`indexUrl`. To do so, because it is final, the base class constructor
delegates to `getIndexUrl()` which is overriden by the v1 updater.
This is required because we want to differentiate between broadcasts
coming from different repo update processes.

Fixes #1054.

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

@@ -2,10 +2,8 @@ package org.fdroid.fdroid;
 
 import android.content.ContentValues;
 import android.content.Context;
-import android.content.Intent;
 import android.net.Uri;
 import android.support.annotation.NonNull;
-import android.support.v4.content.LocalBroadcastManager;
 import android.text.TextUtils;
 import android.util.Log;
 
@@ -61,11 +59,13 @@ public class IndexV1Updater extends RepoUpdater {
     private static final String SIGNED_FILE_NAME = "index-v1.jar";
     public static final String DATA_FILE_NAME = "index-v1.json";
 
-    private final LocalBroadcastManager localBroadcastManager;
-
     public IndexV1Updater(@NonNull Context context, @NonNull Repo repo) {
         super(context, repo);
-        this.localBroadcastManager = LocalBroadcastManager.getInstance(this.context);
+    }
+
+    @Override
+    protected String getIndexUrl(@NonNull Repo repo) {
+        return Uri.parse(repo.address).buildUpon().appendPath(SIGNED_FILE_NAME).build().toString();
     }
 
     /**
@@ -83,19 +83,10 @@ public class IndexV1Updater extends RepoUpdater {
         InputStream indexInputStream = null;
         try {
             // read file name from file
-            final Uri dataUri = Uri.parse(repo.address).buildUpon().appendPath(SIGNED_FILE_NAME).build();
+            final Uri dataUri = Uri.parse(indexUrl);
             downloader = DownloaderFactory.create(context, dataUri.toString());
             downloader.setCacheTag(repo.lastetag);
-            downloader.setListener(new ProgressListener() {
+            downloader.setListener(downloadListener);
-                @Override
-                public void onProgress(URL sourceUrl, int bytesRead, int totalBytes) {
-                    Intent intent = new Intent(Downloader.ACTION_PROGRESS);
-                    intent.setData(dataUri);
-                    intent.putExtra(Downloader.EXTRA_BYTES_READ, bytesRead);
-                    intent.putExtra(Downloader.EXTRA_TOTAL_BYTES, totalBytes);
-                    localBroadcastManager.sendBroadcast(intent);
-                }
-            });
             downloader.download();
             if (downloader.isNotFound()) {
                 return false;
@@ -114,7 +105,7 @@ public class IndexV1Updater extends RepoUpdater {
             JarFile jarFile = new JarFile(downloader.outputFile, true);
             JarEntry indexEntry = (JarEntry) jarFile.getEntry(DATA_FILE_NAME);
             indexInputStream = new ProgressBufferedInputStream(jarFile.getInputStream(indexEntry),
-                    processXmlProgressListener, new URL(repo.address), (int) indexEntry.getSize());
+                    processIndexListener, new URL(repo.address), (int) indexEntry.getSize());
             processIndexV1(indexInputStream, indexEntry, downloader.getCacheTag());
 
         } catch (IOException e) {
@@ -156,6 +147,8 @@ public class IndexV1Updater extends RepoUpdater {
      */
     public void processIndexV1(InputStream indexInputStream, JarEntry indexEntry, String cacheTag)
             throws IOException, UpdateException {
+        Utils.Profiler profiler = new Utils.Profiler(TAG);
+        profiler.log("Starting to process index-v1.json");
         ObjectMapper mapper = getObjectMapperInstance(repo.getId());
         JsonFactory f = mapper.getFactory();
         JsonParser parser = f.createParser(indexInputStream);
@@ -186,6 +179,7 @@ public class IndexV1Updater extends RepoUpdater {
             }
         }
         parser.close(); // ensure resources get cleaned up timely and properly
+        profiler.log("Finished processing index-v1.json. Now verifying certificate...");
 
         if (repoMap == null) {
             return;
@@ -200,7 +194,8 @@ public class IndexV1Updater extends RepoUpdater {
 
         X509Certificate certificate = getSigningCertFromJar(indexEntry);
         verifySigningCertificate(certificate);
-        Utils.debugLog(TAG, "Repo signature verified, saving app metadata to database.");
+
+        profiler.log("Certificate verified. Now saving to database...");
 
         // timestamp is absolutely required
         repo.timestamp = timestamp;
@@ -215,7 +210,9 @@ public class IndexV1Updater extends RepoUpdater {
 
         RepoPersister repoPersister = new RepoPersister(context, repo);
         if (apps != null && apps.length > 0) {
+            int appCount = 0;
             for (App app : apps) {
+                appCount++;
                 List<Apk> apks = null;
                 if (packages != null) {
                     apks = packages.get(app.packageName);
@@ -224,16 +221,25 @@ public class IndexV1Updater extends RepoUpdater {
                     Log.i(TAG, "processIndexV1 empty packages");
                     apks = new ArrayList<Apk>(0);
                 }
+
+                if (appCount % 50 == 0) {
+                    notifyProcessingApps(appCount, apps.length);
+                }
+
                 repoPersister.saveToDb(app, apks);
             }
         }
 
-        // TODO send event saying moving on to committing to db
+        profiler.log("Saved to database, but only a temporary table. Now persisting to database...");
+
+        notifyCommittingToDb();
         ContentValues values = prepareRepoDetailsForSaving(repo.name,
                 repo.description, repo.maxage, repo.version, repo.timestamp, repo.icon,
                 repo.mirrors, cacheTag);
         repoPersister.commit(values);
 
+        profiler.log("Persited to database.");
+
 
         // TODO RepoUpdater.processRepoPushRequests(context, repoPushRequestList);
         Utils.debugLog(TAG, "Repo Push Requests: " + requests);

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

@@ -21,16 +21,15 @@
  */
 
 package org.fdroid.fdroid;
-// TODO move to org.fdroid.fdroid.updater
-// TODO reduce visibility of methods once in .updater package (.e.g tests need it public now)
 
 import android.content.ContentResolver;
 import android.content.ContentValues;
 import android.content.Context;
+import android.content.Intent;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
 import android.support.annotation.NonNull;
-import android.support.annotation.Nullable;
+import android.support.v4.content.LocalBroadcastManager;
 import android.text.TextUtils;
 import android.util.Log;
 
@@ -54,7 +53,6 @@ import org.xml.sax.XMLReader;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.net.MalformedURLException;
 import java.net.URL;
 import java.security.CodeSigner;
 import java.security.cert.Certificate;
@@ -70,6 +68,9 @@ import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.parsers.SAXParser;
 import javax.xml.parsers.SAXParserFactory;
 
+// TODO move to org.fdroid.fdroid.updater
+// TODO reduce visibility of methods once in .updater package (.e.g tests need it public now)
+
 /**
  * Updates the local database with a repository's app/apk metadata and verifying
  * the JAR signature on the file received from the repository. As an overview:
@@ -85,22 +86,42 @@ import javax.xml.parsers.SAXParserFactory;
  * very careful with the changes that you are making!
  */
 public class RepoUpdater {
-    //TODO rename RepoUpdater to IndexV0Updater
     private static final String TAG = "RepoUpdater";
 
-    private final String indexUrl;
+    /**
+     * @see #EXTRA_URL
+     * @see #EXTRA_BYTES_READ
+     * @see #EXTRA_TOTAL_BYTES
+     */
+    public static final String ACTION_DOWNLOADING = "org.fdroid.fdroid.RepoUpdater.ACTION_DOWNLOADING";
+
+    /**
+     * @see #EXTRA_URL
+     * @see #EXTRA_BYTES_READ
+     * @see #EXTRA_TOTAL_BYTES
+     */
+    public static final String ACTION_INDEX_PROCESSING = "org.fdroid.fdroid.RepoUpdater.ACTION_INDEX_PROCESSING";
+
+    /**
+     * @see #EXTRA_URL
+     * @see #EXTRA_APPS_SAVED
+     * @see #EXTRA_TOTAL_APPS
+     */
+    public static final String ACTION_SAVING_APPS = "org.fdroid.fdroid.RepoUpdater.ACTION_SAVING_APPS";
+
+    public static final String EXTRA_URL = "URL";
+    public static final String EXTRA_BYTES_READ = "BYTES_READ";
+    public static final String EXTRA_TOTAL_BYTES = "TOTAL_BYTES";
+    public static final String EXTRA_APPS_SAVED = "APPS_SAVED";
+    public static final String EXTRA_TOTAL_APPS = "TOTAL_APPS";
+
+    final String indexUrl;
 
     @NonNull
     final Context context;
     @NonNull
     final Repo repo;
     boolean hasChanged;
-
-    @Nullable
-    ProgressListener downloadProgressListener;
-    ProgressListener committingProgressListener;
-    ProgressListener processXmlProgressListener;
-
     private String cacheTag;
     private X509Certificate signingCertFromJar;
 
@@ -118,25 +139,16 @@ public class RepoUpdater {
         this.context = context;
         this.repo = repo;
         this.persister = new RepoPersister(context, repo);
+        this.indexUrl = getIndexUrl(repo);
+    }
 
+    protected String getIndexUrl(@NonNull Repo repo) {
         String url = repo.address + "/index.jar";
         String versionName = Utils.getVersionName(context);
         if (versionName != null) {
             url += "?client_version=" + versionName;
         }
-        this.indexUrl = url;
+        return url;
-    }
-
-    public void setDownloadProgressListener(ProgressListener progressListener) {
-        this.downloadProgressListener = progressListener;
-    }
-
-    public void setProcessXmlProgressListener(ProgressListener progressListener) {
-        this.processXmlProgressListener = progressListener;
-    }
-
-    public void setCommittingProgressListener(ProgressListener progressListener) {
-        this.committingProgressListener = progressListener;
     }
 
     public boolean hasChanged() {
@@ -148,7 +160,7 @@ public class RepoUpdater {
         try {
             downloader = DownloaderFactory.create(context, indexUrl);
             downloader.setCacheTag(repo.lastetag);
-            downloader.setListener(downloadProgressListener);
+            downloader.setListener(downloadListener);
             downloader.download();
 
             if (downloader.isCached()) {
@@ -238,7 +250,7 @@ public class RepoUpdater {
             JarFile jarFile = new JarFile(downloadedFile, true);
             JarEntry indexEntry = (JarEntry) jarFile.getEntry("index.xml");
             indexInputStream = new ProgressBufferedInputStream(jarFile.getInputStream(indexEntry),
-                    processXmlProgressListener, new URL(repo.address), (int) indexEntry.getSize());
+                    processIndexListener, new URL(repo.address), (int) indexEntry.getSize());
 
             // Process the index...
             SAXParserFactory factory = SAXParserFactory.newInstance();
@@ -274,16 +286,43 @@ public class RepoUpdater {
         }
     }
 
+    protected final ProgressListener downloadListener = new ProgressListener() {
+        @Override
+        public void onProgress(URL sourceUrl, int bytesRead, int totalBytes) {
+            Intent intent = new Intent(ACTION_DOWNLOADING);
+            intent.putExtra(EXTRA_URL, indexUrl);
+            intent.putExtra(EXTRA_BYTES_READ, bytesRead);
+            intent.putExtra(EXTRA_TOTAL_BYTES, totalBytes);
+            LocalBroadcastManager.getInstance(context).sendBroadcast(intent);
+        }
+    };
+
+    protected final ProgressListener processIndexListener = new ProgressListener() {
+        @Override
+        public void onProgress(URL sourceUrl, int bytesRead, int totalBytes) {
+            Intent intent = new Intent(ACTION_INDEX_PROCESSING);
+            intent.putExtra(EXTRA_URL, indexUrl);
+            intent.putExtra(EXTRA_BYTES_READ, bytesRead);
+            intent.putExtra(EXTRA_TOTAL_BYTES, totalBytes);
+            LocalBroadcastManager.getInstance(context).sendBroadcast(intent);
+        }
+    };
+
+    protected void notifyProcessingApps(int appsSaved, int totalBytes) {
+        Intent intent = new Intent(ACTION_SAVING_APPS);
+        intent.putExtra(EXTRA_URL, indexUrl);
+        intent.putExtra(EXTRA_APPS_SAVED, appsSaved);
+        intent.putExtra(EXTRA_TOTAL_APPS, totalBytes);
+        LocalBroadcastManager.getInstance(context).sendBroadcast(intent);
+    }
+
+    protected void notifyCommittingToDb() {
+        notifyProcessingApps(0, -1);
+    }
+
     private void commitToDb() throws UpdateException {
         Log.i(TAG, "Repo signature verified, saving app metadata to database.");
-        if (committingProgressListener != null) {
+        notifyCommittingToDb();
-            try {
-                //TODO this should be an event, not a progress listener
-                committingProgressListener.onProgress(new URL(indexUrl), 0, -1);
-            } catch (MalformedURLException e) {
-                e.printStackTrace();
-            }
-        }
         persister.commit(repoDetailsToSave);
     }
 
@@ -479,7 +518,7 @@ public class RepoUpdater {
                 // TODO: In the future, this needs to be able to specify which repository to get
                 // the package from. Better yet, we should be able to specify the hash of a package
                 // to install (especially when we move to using hashes more as identifiers than we
-                // do righ tnow).
+                // do right now).
                 App app = AppProvider.Helper.findHighestPriorityMetadata(cr, packageName);
                 if (app == null) {
                     Utils.debugLog(TAG, packageName + " not in local database, ignoring request to"

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

@@ -51,7 +51,6 @@ import org.fdroid.fdroid.data.Schema;
 import org.fdroid.fdroid.installer.InstallManagerService;
 import org.fdroid.fdroid.views.main.MainActivity;
 
-import java.net.URL;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -408,12 +407,15 @@ public class UpdateService extends IntentService {
 
                 try {
                     RepoUpdater updater = new IndexV1Updater(this, repo);
-                    //TODO setProgressListeners(updater);
+                    UpdateBroadcastReceivers receivers = new UpdateBroadcastReceivers(updater);
                     if (Preferences.get().isForceOldIndexEnabled() || !updater.update()) {
+                        receivers.unregisterReceivers();
                         updater = new RepoUpdater(getBaseContext(), repo);
-                        setProgressListeners(updater);
+                        receivers = new UpdateBroadcastReceivers(updater);
                         updater.update();
                     }
+                    receivers.unregisterReceivers();
+
                     if (updater.hasChanged()) {
                         updatedRepos++;
                         changes = true;
@@ -502,52 +504,116 @@ public class UpdateService extends IntentService {
     }
 
     /**
-     * Set up the various {@link ProgressListener}s needed to get feedback to the UI.
+     * Creates three broadcast receivers (downloading, processing index, saving apps) and listens
-     * Note: {@code ProgressListener}s do not need to be unregistered, they can just
+     * for relevant broadcasts from them. Calling {@link #unregisterReceivers()} will unregister
-     * be set again for each download.
+     * all three receivers. If you neglect to unregister them, then they will continually receive
+     * events into the future.
+     *
+     * For each broadcast it receives, it checks the URL of the sender, because it may be possible
+     * to have, e.g. a swap and a regular repo update happening at once.
      */
-    private void setProgressListeners(RepoUpdater updater) {
+    private class UpdateBroadcastReceivers {
-        updater.setDownloadProgressListener(new ProgressListener() {
-            @Override
-            public void onProgress(URL sourceUrl, int bytesRead, int totalBytes) {
-                Log.i(TAG, "downloadProgressReceiver " + sourceUrl);
-                String downloadedSizeFriendly = Utils.getFriendlySize(bytesRead);
-                int percent = -1;
-                if (totalBytes > 0) {
-                    percent = (int) ((double) bytesRead / totalBytes * 100);
-                }
-                String message;
-                if (totalBytes == -1) {
-                    message = getString(R.string.status_download_unknown_size, sourceUrl, downloadedSizeFriendly);
-                    percent = -1;
-                } else {
-                    String totalSizeFriendly = Utils.getFriendlySize(totalBytes);
-                    message = getString(R.string.status_download, sourceUrl, downloadedSizeFriendly, totalSizeFriendly, percent);
-                }
-                sendStatus(getApplicationContext(), STATUS_INFO, message, percent);
-            }
-        });
 
-        updater.setProcessXmlProgressListener(new ProgressListener() {
+        private final BroadcastReceiver downloading;
-            @Override
+        private final BroadcastReceiver processingIndex;
-            public void onProgress(URL sourceUrl, int bytesRead, int totalBytes) {
+        private final BroadcastReceiver savingApps;
-                String downloadedSize = Utils.getFriendlySize(bytesRead);
-                String totalSize = Utils.getFriendlySize(totalBytes);
-                int percent = -1;
-                if (totalBytes > 0) {
-                    percent = (int) ((double) bytesRead / totalBytes * 100);
-                }
-                String message = getString(R.string.status_processing_xml_percent, sourceUrl, downloadedSize, totalSize, percent);
-                sendStatus(getApplicationContext(), STATUS_INFO, message, percent);
-            }
-        });
 
-        updater.setCommittingProgressListener(new ProgressListener() {
+        UpdateBroadcastReceivers(final RepoUpdater updater) {
-            @Override
+            downloading = new BroadcastReceiver() {
-            public void onProgress(URL sourceUrl, int bytesRead, int totalBytes) {
+                @Override
-                String message = getString(R.string.status_inserting_apps);
+                public void onReceive(Context context, Intent intent) {
-                sendStatus(getApplicationContext(), STATUS_INFO, message);
+                    String url = intent.getStringExtra(RepoUpdater.EXTRA_URL);
-            }
+                    if (TextUtils.equals(url, updater.indexUrl)) {
-        });
+                        reportDownloadProgress(updater,
+                                intent.getIntExtra(RepoUpdater.EXTRA_BYTES_READ, 0),
+                                intent.getIntExtra(RepoUpdater.EXTRA_TOTAL_BYTES, 0));
+                    }
+                }
+            };
+
+            processingIndex = new BroadcastReceiver() {
+                @Override
+                public void onReceive(Context context, Intent intent) {
+                    String url = intent.getStringExtra(RepoUpdater.EXTRA_URL);
+                    if (TextUtils.equals(url, updater.indexUrl)) {
+                        reportProcessIndexProgress(updater,
+                                intent.getIntExtra(RepoUpdater.EXTRA_BYTES_READ, 0),
+                                intent.getIntExtra(RepoUpdater.EXTRA_TOTAL_BYTES, 0));
+                    }
+                }
+            };
+
+            savingApps = new BroadcastReceiver() {
+                @Override
+                public void onReceive(Context context, Intent intent) {
+                    String url = intent.getStringExtra(RepoUpdater.EXTRA_URL);
+                    if (TextUtils.equals(url, updater.indexUrl)) {
+                        reportProcessingAppsProgress(updater,
+                                intent.getIntExtra(RepoUpdater.EXTRA_APPS_SAVED, 0),
+                                intent.getIntExtra(RepoUpdater.EXTRA_TOTAL_APPS, 0));
+                    }
+                }
+            };
+
+            LocalBroadcastManager manager = LocalBroadcastManager.getInstance(UpdateService.this);
+            manager.registerReceiver(savingApps, new IntentFilter(RepoUpdater.ACTION_SAVING_APPS));
+            manager.registerReceiver(processingIndex, new IntentFilter(RepoUpdater.ACTION_INDEX_PROCESSING));
+            manager.registerReceiver(downloading, new IntentFilter(RepoUpdater.ACTION_DOWNLOADING));
+        }
+
+        public void unregisterReceivers() {
+            LocalBroadcastManager manager = LocalBroadcastManager.getInstance(UpdateService.this);
+            manager.unregisterReceiver(savingApps);
+            manager.unregisterReceiver(processingIndex);
+            manager.unregisterReceiver(downloading);
+        }
+
+    }
+
+    private void reportDownloadProgress(RepoUpdater updater, int bytesRead, int totalBytes) {
+        Utils.debugLog(TAG, "Downloading " + updater.indexUrl + "(" + bytesRead + "/" + totalBytes + ")");
+        String downloadedSizeFriendly = Utils.getFriendlySize(bytesRead);
+        int percent = -1;
+        if (totalBytes > 0) {
+            percent = (int) ((double) bytesRead / totalBytes * 100);
+        }
+        String message;
+        if (totalBytes == -1) {
+            message = getString(R.string.status_download_unknown_size, updater.indexUrl, downloadedSizeFriendly);
+            percent = -1;
+        } else {
+            String totalSizeFriendly = Utils.getFriendlySize(totalBytes);
+            message = getString(R.string.status_download, updater.indexUrl, downloadedSizeFriendly, totalSizeFriendly, percent);
+        }
+        sendStatus(getApplicationContext(), STATUS_INFO, message, percent);
+    }
+
+    private void reportProcessIndexProgress(RepoUpdater updater, int bytesRead, int totalBytes) {
+        Utils.debugLog(TAG, "Processing " + updater.indexUrl + "(" + bytesRead + "/" + totalBytes + ")");
+        String downloadedSize = Utils.getFriendlySize(bytesRead);
+        String totalSize = Utils.getFriendlySize(totalBytes);
+        int percent = -1;
+        if (totalBytes > 0) {
+            percent = (int) ((double) bytesRead / totalBytes * 100);
+        }
+        String message = getString(R.string.status_processing_xml_percent, updater.indexUrl, downloadedSize, totalSize, percent);
+        sendStatus(getApplicationContext(), STATUS_INFO, message, percent);
+    }
+
+    /**
+     * If an updater is unable to know how many apps it has to process (i.e. it is streaming apps to the database or
+     * performing a large database query which touches all apps, but is unable to report progress), then it call this
+     * listener with `totalBytes = 0`. Doing so will result in a message of "Saving app details" sent to the user. If
+     * you know how many apps you have processed, then a message of "Saving app details (x/total)" is displayed.
+     */
+    private void reportProcessingAppsProgress(RepoUpdater updater, int appsSaved, int totalApps) {
+        Utils.debugLog(TAG, "Committing " + updater.indexUrl + "(" + appsSaved + "/" + totalApps + ")");
+        String message;
+        if (totalApps > 0) {
+            message = getString(R.string.status_inserting_x_apps, appsSaved, totalApps, updater.indexUrl);
+        } else {
+            message = getString(R.string.status_inserting_apps);
+        }
+        sendStatus(getApplicationContext(), STATUS_INFO, message);
     }
 }

app/src/main/res/values/strings.xml

@@ -243,6 +243,7 @@
     <string name="status_processing_xml_percent">Processing %2$s / %3$s (%4$d%%) from %1$s</string>
     <string name="status_connecting_to_repo">Connecting to\n%1$s</string>
     <string name="status_inserting_apps">Saving app details</string>
+    <string name="status_inserting_x_apps">Saving app details (%1$d/%2$d) from %3$s</string>
     <string name="repos_unchanged">All repositories are up to date</string>
     <string name="all_other_repos_fine">All other repos didn\'t create errors.</string>
     <string name="global_error_updating_repos">Error during update: %s</string>