Karl/BobStore
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

replace QUEUE_WHATS hack with urlString.hashCode() as ID

Browse Source 
There is already a method for reproducibly generating an int based on
a the contents of a String, thanks to @pserwylo for finding it! So
urlString.hashCode() is used as the ID for Notifications and the
DownloaderService queue (msg.what).  This entirely replaces the
QUEUE_WHATS hack.  Both requestCode and msg.what just need to be
unique int, and that value can always be generated by the urlString.

This also fixes a bug preventing removing correct URL from Downloader queue.
b66810944fec802aa119c0e5ec8b7875930a2c22 made a change that breaks removing
the correct item from the queue. The `what` value must be fetched based on
urlString and fed to serviceHandler.removeMessages(what). The commit made
it use the `what` value from the last item that was queued. Those will
often be different things.

This also removes all stopForeground(true) calls except for the one in
onDestroy().  The nature of an IntentService, which DownloaderService
is basically a copy of, is to quit running once it is no longer
processing Intents.  That is also the time to remove the notification.
This commit is contained in:
Hans-Christoph Steiner 2016-05-03 12:54:42 +02:00
parent 18b3a05806
commit 44b703a7d2
4 changed files with 34 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
app/src/main/java/org/fdroid/fdroid/AppDetails.java
View File

@ -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("");
}
}
}

10
app/src/main/java/org/fdroid/fdroid/Utils.java
View File

@ -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<String> {
private final String value;

2
app/src/main/java/org/fdroid/fdroid/installer/Installer.java
View File

@ -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) {

81
app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java
View File

@ -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.
* <p/>
* <p>This "work queue processor" pattern is commonly used to offload tasks
* <p>
* 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.
* <p/>
* <p>All requests are handled on a single worker thread -- they may take as
* <p>
* 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.
* <p/>
* <div class="special reference">
* <h3>Developer Guides</h3>
* <p>For a detailed discussion about how to create services, read the
* <a href="{@docRoot}guide/topics/fundamentals/services.html">Services</a> developer guide.</p>
* </div>
* <p>
* 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<String, Integer> 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 {
* <p/>
* 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 {
* <p/>
* 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);