From df20d2df8d5bf15f03df4797caf07502422ced36 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 26 Jun 2017 16:34:07 +1000 Subject: [PATCH 1/3] 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. --- .../org/fdroid/fdroid/IndexV1Updater.java | 44 ++--- .../java/org/fdroid/fdroid/RepoUpdater.java | 111 ++++++++---- .../java/org/fdroid/fdroid/UpdateService.java | 160 +++++++++++++----- app/src/main/res/values/strings.xml | 1 + 4 files changed, 214 insertions(+), 102 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java b/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java index 5118dbf90..7b4d36a5e 100644 --- a/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java +++ b/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() { - @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.setListener(downloadListener); 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 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(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); diff --git a/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java b/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java index e2abe0df2..b74c530b6 100644 --- a/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java +++ b/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; - } - - public void setDownloadProgressListener(ProgressListener progressListener) { - this.downloadProgressListener = progressListener; - } - - public void setProcessXmlProgressListener(ProgressListener progressListener) { - this.processXmlProgressListener = progressListener; - } - - public void setCommittingProgressListener(ProgressListener progressListener) { - this.committingProgressListener = progressListener; + return url; } 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) { - try { - //TODO this should be an event, not a progress listener - committingProgressListener.onProgress(new URL(indexUrl), 0, -1); - } catch (MalformedURLException e) { - e.printStackTrace(); - } - } + notifyCommittingToDb(); 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" diff --git a/app/src/main/java/org/fdroid/fdroid/UpdateService.java b/app/src/main/java/org/fdroid/fdroid/UpdateService.java index db0b49e1e..b4a997609 100644 --- a/app/src/main/java/org/fdroid/fdroid/UpdateService.java +++ b/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. - * Note: {@code ProgressListener}s do not need to be unregistered, they can just - * be set again for each download. + * Creates three broadcast receivers (downloading, processing index, saving apps) and listens + * for relevant broadcasts from them. Calling {@link #unregisterReceivers()} will unregister + * 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) { - 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); - } - }); + private class UpdateBroadcastReceivers { - updater.setProcessXmlProgressListener(new ProgressListener() { - @Override - public void onProgress(URL sourceUrl, int bytesRead, int 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, sourceUrl, downloadedSize, totalSize, percent); - sendStatus(getApplicationContext(), STATUS_INFO, message, percent); - } - }); + private final BroadcastReceiver downloading; + private final BroadcastReceiver processingIndex; + private final BroadcastReceiver savingApps; - updater.setCommittingProgressListener(new ProgressListener() { - @Override - public void onProgress(URL sourceUrl, int bytesRead, int totalBytes) { - String message = getString(R.string.status_inserting_apps); - sendStatus(getApplicationContext(), STATUS_INFO, message); - } - }); + UpdateBroadcastReceivers(final RepoUpdater updater) { + downloading = new BroadcastReceiver() { + @Override + public void onReceive(Context context, Intent intent) { + 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); } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index d1770033e..4a8e56659 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -243,6 +243,7 @@ Processing %2$s / %3$s (%4$d%%) from %1$s Connecting to\n%1$s Saving app details + Saving app details (%1$d/%2$d) from %3$s All repositories are up to date All other repos didn\'t create errors. Error during update: %s From b7a20bbf01f7cf12e5f0ebb02960354e1bf261a6 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 3 Jul 2017 08:59:00 +1000 Subject: [PATCH 2/3] Remove one layer of indirection in LocalBraodcasts for updating repo. This creates a hard dependence between `RepoUpdater` and `UpdateService`. However this could be trivially extracted by moving the helper methods from `UpdateService` to `RepoUpdater`, and making the broadcasts more "repo updater" oriented. That would also require changing the broadcasts which `UpdateService` listens for. --- .../java/org/fdroid/fdroid/RepoUpdater.java | 49 +-------- .../java/org/fdroid/fdroid/UpdateService.java | 99 +++---------------- 2 files changed, 18 insertions(+), 130 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java b/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java index b74c530b6..b63424f70 100644 --- a/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java +++ b/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java @@ -25,11 +25,9 @@ package org.fdroid.fdroid; 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.v4.content.LocalBroadcastManager; import android.text.TextUtils; import android.util.Log; @@ -88,33 +86,6 @@ import javax.xml.parsers.SAXParserFactory; public class RepoUpdater { private static final String TAG = "RepoUpdater"; - /** - * @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 @@ -289,31 +260,19 @@ 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); + UpdateService.reportDownloadProgress(context, RepoUpdater.this, bytesRead, totalBytes); } }; 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); + UpdateService.reportProcessIndexProgress(context, RepoUpdater.this, bytesRead, totalBytes); } }; - 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 notifyProcessingApps(int appsSaved, int totalApps) { + UpdateService.reportProcessingAppsProgress(context, this, appsSaved, totalApps); } protected void notifyCommittingToDb() { diff --git a/app/src/main/java/org/fdroid/fdroid/UpdateService.java b/app/src/main/java/org/fdroid/fdroid/UpdateService.java index b4a997609..701d5ee43 100644 --- a/app/src/main/java/org/fdroid/fdroid/UpdateService.java +++ b/app/src/main/java/org/fdroid/fdroid/UpdateService.java @@ -177,15 +177,15 @@ public class UpdateService extends IntentService { LocalBroadcastManager.getInstance(this).unregisterReceiver(updateStatusReceiver); } - private static void sendStatus(Context context, int statusCode) { + public static void sendStatus(Context context, int statusCode) { sendStatus(context, statusCode, null, -1); } - private static void sendStatus(Context context, int statusCode, String message) { + public static void sendStatus(Context context, int statusCode, String message) { sendStatus(context, statusCode, message, -1); } - private static void sendStatus(Context context, int statusCode, String message, int progress) { + public static void sendStatus(Context context, int statusCode, String message, int progress) { Intent intent = new Intent(LOCAL_ACTION_STATUS); intent.putExtra(EXTRA_STATUS_CODE, statusCode); if (!TextUtils.isEmpty(message)) { @@ -407,14 +407,10 @@ public class UpdateService extends IntentService { try { RepoUpdater updater = new IndexV1Updater(this, repo); - UpdateBroadcastReceivers receivers = new UpdateBroadcastReceivers(updater); if (Preferences.get().isForceOldIndexEnabled() || !updater.update()) { - receivers.unregisterReceivers(); updater = new RepoUpdater(getBaseContext(), repo); - receivers = new UpdateBroadcastReceivers(updater); updater.update(); } - receivers.unregisterReceivers(); if (updater.hasChanged()) { updatedRepos++; @@ -503,74 +499,7 @@ public class UpdateService extends IntentService { } } - /** - * Creates three broadcast receivers (downloading, processing index, saving apps) and listens - * for relevant broadcasts from them. Calling {@link #unregisterReceivers()} will unregister - * 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 class UpdateBroadcastReceivers { - - private final BroadcastReceiver downloading; - private final BroadcastReceiver processingIndex; - private final BroadcastReceiver savingApps; - - UpdateBroadcastReceivers(final RepoUpdater updater) { - downloading = new BroadcastReceiver() { - @Override - public void onReceive(Context context, Intent intent) { - 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) { + public static void reportDownloadProgress(Context context, RepoUpdater updater, int bytesRead, int totalBytes) { Utils.debugLog(TAG, "Downloading " + updater.indexUrl + "(" + bytesRead + "/" + totalBytes + ")"); String downloadedSizeFriendly = Utils.getFriendlySize(bytesRead); int percent = -1; @@ -579,16 +508,16 @@ public class UpdateService extends IntentService { } String message; if (totalBytes == -1) { - message = getString(R.string.status_download_unknown_size, updater.indexUrl, downloadedSizeFriendly); + message = context.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); + message = context.getString(R.string.status_download, updater.indexUrl, downloadedSizeFriendly, totalSizeFriendly, percent); } - sendStatus(getApplicationContext(), STATUS_INFO, message, percent); + sendStatus(context, STATUS_INFO, message, percent); } - private void reportProcessIndexProgress(RepoUpdater updater, int bytesRead, int totalBytes) { + public static void reportProcessIndexProgress(Context context, RepoUpdater updater, int bytesRead, int totalBytes) { Utils.debugLog(TAG, "Processing " + updater.indexUrl + "(" + bytesRead + "/" + totalBytes + ")"); String downloadedSize = Utils.getFriendlySize(bytesRead); String totalSize = Utils.getFriendlySize(totalBytes); @@ -596,8 +525,8 @@ public class UpdateService extends IntentService { 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); + String message = context.getString(R.string.status_processing_xml_percent, updater.indexUrl, downloadedSize, totalSize, percent); + sendStatus(context, STATUS_INFO, message, percent); } /** @@ -606,14 +535,14 @@ public class UpdateService extends IntentService { * 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) { + public static void reportProcessingAppsProgress(Context context, 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); + message = context.getString(R.string.status_inserting_x_apps, appsSaved, totalApps, updater.indexUrl); } else { - message = getString(R.string.status_inserting_apps); + message = context.getString(R.string.status_inserting_apps); } - sendStatus(getApplicationContext(), STATUS_INFO, message); + sendStatus(context, STATUS_INFO, message); } } From 3d195c9dc0684da633de4f4177469deb4621c7bb Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 3 Jul 2017 09:03:28 +1000 Subject: [PATCH 3/3] Report progress correctly Was previously sending an indeterminate progress event, when we actually knew how long was remaining. --- app/src/main/java/org/fdroid/fdroid/UpdateService.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/UpdateService.java b/app/src/main/java/org/fdroid/fdroid/UpdateService.java index 701d5ee43..e6a02303f 100644 --- a/app/src/main/java/org/fdroid/fdroid/UpdateService.java +++ b/app/src/main/java/org/fdroid/fdroid/UpdateService.java @@ -537,12 +537,12 @@ public class UpdateService extends IntentService { */ public static void reportProcessingAppsProgress(Context context, RepoUpdater updater, int appsSaved, int totalApps) { Utils.debugLog(TAG, "Committing " + updater.indexUrl + "(" + appsSaved + "/" + totalApps + ")"); - String message; if (totalApps > 0) { - message = context.getString(R.string.status_inserting_x_apps, appsSaved, totalApps, updater.indexUrl); + String message = context.getString(R.string.status_inserting_x_apps, appsSaved, totalApps, updater.indexUrl); + sendStatus(context, STATUS_INFO, message, (int) ((double) appsSaved / totalApps * 100)); } else { - message = context.getString(R.string.status_inserting_apps); + String message = context.getString(R.string.status_inserting_apps); + sendStatus(context, STATUS_INFO, message); } - sendStatus(context, STATUS_INFO, message); } }