diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index 8142f62e4..88d17a270 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -1443,10 +1443,11 @@ public class AppDetails extends AppCompatActivity { if (appDetails.activeDownloadUrlString != null) { // We don't actually know what the current progress is, so this will show an indeterminate // progress bar until the first progress/complete event we receive. - final String message = DownloaderService.isQueued(appDetails.activeDownloadUrlString) - ? getString(R.string.download_pending) - : ""; - showIndeterminateProgress(message); + if (DownloaderService.isQueuedOrActive(appDetails.activeDownloadUrlString)) { + showIndeterminateProgress(getString(R.string.download_pending)); + } else { + showIndeterminateProgress(""); + } } } diff --git a/app/src/main/java/org/fdroid/fdroid/Utils.java b/app/src/main/java/org/fdroid/fdroid/Utils.java index 2e7ceb163..d0dfa9b1a 100644 --- a/app/src/main/java/org/fdroid/fdroid/Utils.java +++ b/app/src/main/java/org/fdroid/fdroid/Utils.java @@ -67,7 +67,6 @@ import java.util.Formatter; import java.util.Iterator; import java.util.List; import java.util.Locale; -import java.util.zip.Adler32; public final class Utils { @@ -439,15 +438,6 @@ public final class Utils { return repoAddress + "/" + apk.apkName.replace(" ", "%20"); } - /** - * This generates a unique, reproducible ID for notifications related to {@code urlString} - */ - public static int getApkUrlNotificationId(String urlString) { - Adler32 checksum = new Adler32(); - checksum.update(urlString.getBytes()); - return (int) checksum.getValue(); - } - public static final class CommaSeparatedList implements Iterable { private final String value; diff --git a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java index 8a2e21ed6..41266f021 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java @@ -227,7 +227,7 @@ public abstract class Installer { NotificationManager nm = (NotificationManager) mContext.getSystemService(Context.NOTIFICATION_SERVICE); - nm.cancel(Utils.getApkUrlNotificationId(urlString)); + nm.cancel(urlString.hashCode()); } catch (NumberFormatException | NoSuchAlgorithmException | IOException e) { throw new InstallFailedException(e); } catch (ClassCastException e) { diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java index d7ddc2398..d2186582d 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java @@ -53,7 +53,6 @@ import org.fdroid.fdroid.data.SanitizedFile; import java.io.File; import java.io.IOException; import java.net.URL; -import java.util.HashMap; /** * DownloaderService is a service that handles asynchronous download requests @@ -61,24 +60,28 @@ import java.util.HashMap; * through {@link android.content.Context#startService(Intent)} calls; the * service is started as needed, handles each Intent in turn using a worker * thread, and stops itself when it runs out of work. - *

- *

This "work queue processor" pattern is commonly used to offload tasks + *

+ * This "work queue processor" pattern is commonly used to offload tasks * from an application's main thread. The DownloaderService class exists to * simplify this pattern and take care of the mechanics. DownloaderService * will receive the Intents, launch a worker thread, and stop the service as * appropriate. - *

- *

All requests are handled on a single worker thread -- they may take as + *

+ * All requests are handled on a single worker thread -- they may take as * long as necessary (and will not block the application's main loop), but * only one request will be processed at a time. - *

- *

- *

Developer Guides

- *

For a detailed discussion about how to create services, read the - * Services developer guide.

- *
+ *

+ * The full URL for the file to download is also used as the unique ID to + * represent the download itself throughout F-Droid. This follows the model + * of {@link Intent#setData(Uri)}, where the core data of an {@code Intent} is + * a {@code Uri}. For places that need an {@code int} ID, + * {@link String#hashCode()} should be used to get a reproducible, unique {@code int} + * from any {@code urlString}. The full URL is guaranteed to be unique since + * it points to a file on a filesystem. This is more important with media files + * than with APKs since there is not reliable standard for a unique ID for + * media files, unlike APKs with {@code packageName} and {@code versionCode}. * - * @see android.os.AsyncTask + * @see android.app.IntentService */ public class DownloaderService extends Service { private static final String TAG = "DownloaderService"; @@ -95,9 +98,6 @@ public class DownloaderService extends Service { private static volatile Downloader downloader; private LocalBroadcastManager localBroadcastManager; - private static final HashMap QUEUE_WHATS = new HashMap<>(); - private int what; - private final class ServiceHandler extends Handler { ServiceHandler(Looper looper) { super(looper); @@ -134,26 +134,21 @@ public class DownloaderService extends Service { } if (ACTION_CANCEL.equals(intent.getAction())) { Utils.debugLog(TAG, "Cancelling download of " + uriString); - if (isQueued(uriString)) { - serviceHandler.removeMessages(what); + Integer whatToRemove = uriString.hashCode(); + if (serviceHandler.hasMessages(whatToRemove)) { + serviceHandler.removeMessages(whatToRemove); } else if (isActive(uriString)) { downloader.cancelDownload(); } else { Log.e(TAG, "ACTION_CANCEL called on something not queued or running"); } - - QUEUE_WHATS.remove(uriString); - if (isQueueEmpty()) { - stopForeground(true); - } } else if (ACTION_QUEUE.equals(intent.getAction())) { Message msg = serviceHandler.obtainMessage(); msg.arg1 = startId; msg.obj = intent; - msg.what = what++; + msg.what = uriString.hashCode(); serviceHandler.sendMessage(msg); - QUEUE_WHATS.put(uriString, msg.what); - Utils.debugLog(TAG, "Queued download of " + uriString + ". Now " + QUEUE_WHATS.size() + " downloads in the queue"); + Utils.debugLog(TAG, "Queued download of " + uriString); } else { Log.e(TAG, "Received Intent with unknown action: " + intent); } @@ -308,10 +303,6 @@ public class DownloaderService extends Service { if (downloader != null) { downloader.close(); } - // May have already been removed in response to a cancel intent, but that wont cause - // problems if we ask to remove it again. - QUEUE_WHATS.remove(uri.toString()); - stopForeground(true); } downloader = null; } @@ -330,16 +321,16 @@ public class DownloaderService extends Service { title = String.format(getString(R.string.tap_to_install_format), app.name); } - int requestCode = Utils.getApkUrlNotificationId(urlString); + int downloadUrlId = urlString.hashCode(); NotificationCompat.Builder builder = new NotificationCompat.Builder(this) .setAutoCancel(true) .setContentTitle(title) .setSmallIcon(android.R.drawable.stat_sys_download_done) - .setContentIntent(createAppDetailsIntent(requestCode, packageName)) + .setContentIntent(createAppDetailsIntent(downloadUrlId, packageName)) .setContentText(getString(R.string.tap_to_install)); NotificationManager nm = (NotificationManager) getSystemService(NOTIFICATION_SERVICE); - nm.notify(Utils.getApkUrlNotificationId(urlString), builder.build()); + nm.notify(downloadUrlId, builder.build()); } private void sendBroadcast(Uri uri, String action, File file) { @@ -361,7 +352,7 @@ public class DownloaderService extends Service { *

* All notifications are sent as an {@link Intent} via local broadcasts to be received by * - * @param context + * @param context this app's {@link Context} * @param packageName The packageName of the app being downloaded * @param urlString The URL to add to the download queue * @see #cancel(Context, String) @@ -382,7 +373,7 @@ public class DownloaderService extends Service { *

* All notifications are sent as an {@link Intent} via local broadcasts to be received by * - * @param context + * @param context this app's {@link Context} * @param urlString The URL to remove from the download queue * @see #queue(Context, String, String) */ @@ -398,34 +389,19 @@ public class DownloaderService extends Service { * Check if a URL is waiting in the queue for downloading or if actively being downloaded. * This is useful for checking whether to re-register {@link android.content.BroadcastReceiver}s * in {@link android.app.Activity#onResume()}. - * - * @see DownloaderService#isQueued(String) - * @see DownloaderService#isActive(String) */ public static boolean isQueuedOrActive(String urlString) { - return isQueued(urlString) || isActive(urlString); - } - - public static boolean isQueueEmpty() { - return QUEUE_WHATS.isEmpty(); - } - - /** - * Check if a URL is waiting in the queue for downloading. - */ - public static boolean isQueued(String urlString) { - if (TextUtils.isEmpty(urlString)) { + if (TextUtils.isEmpty(urlString)) { //NOPMD - suggests unreadable format return false; } - Integer what = QUEUE_WHATS.get(urlString); - return what != null && serviceHandler.hasMessages(what); + return serviceHandler.hasMessages(urlString.hashCode()) || isActive(urlString); } /** * Check if a URL is actively being downloaded. */ public static boolean isActive(String urlString) { - return downloader != null && QUEUE_WHATS.containsKey(urlString) && TextUtils.equals(urlString, downloader.sourceUrl.toString()); + return downloader != null && TextUtils.equals(urlString, downloader.sourceUrl.toString()); } /** @@ -434,7 +410,6 @@ public class DownloaderService extends Service { * @param urlString The full file URL to match. * @param action {@link Downloader#ACTION_STARTED}, {@link Downloader#ACTION_PROGRESS}, * {@link Downloader#ACTION_INTERRUPTED}, or {@link Downloader#ACTION_COMPLETE}, - * @return */ public static IntentFilter getIntentFilter(String urlString, String action) { Uri uri = Uri.parse(urlString);