Merge branch 'downloaderservice-fixes' into 'master'

DownloaderService fixes

This aims to simplify the `DownloaderService` a bunch to make it easier to understand.  It also fixes some bugs with the download queue and related progress reports.

This is groundwork for the upcoming `InstallManagerService` which will manage the whole process of installing an app, from checking the cache, to downloading, to running the install process.

See merge request !273
This commit is contained in:
Peter Serwylo 2016-05-10 21:00:54 +00:00
commit 169346ce76
9 changed files with 116 additions and 246 deletions

View File

@ -1,41 +0,0 @@
package org.fdroid.fdroid.net;
import android.content.BroadcastReceiver;
import android.content.Context;
import android.content.Intent;
import android.content.IntentFilter;
import android.support.v4.content.LocalBroadcastManager;
import android.test.ServiceTestCase;
import android.util.Log;
@SuppressWarnings("PMD") // TODO port this to JUnit 4 semantics
public class DownloaderServiceTest extends ServiceTestCase<DownloaderService> {
public static final String TAG = "DownloaderServiceTest";
String[] urls = {
"https://en.wikipedia.org/wiki/Index.html",
"https://mirrors.kernel.org/debian/dists/stable/Release",
"https://f-droid.org/archive/de.we.acaldav_5.apk",
// sites that use SNI for HTTPS
"https://guardianproject.info/fdroid/repo/index.jar",
};
public DownloaderServiceTest() {
super(DownloaderService.class);
}
public void testQueueingDownload() throws InterruptedException {
LocalBroadcastManager localBroadcastManager = LocalBroadcastManager.getInstance(getContext());
localBroadcastManager.registerReceiver(new BroadcastReceiver() {
@Override
public void onReceive(Context context, Intent intent) {
Log.i(TAG, "onReceive " + intent);
}
}, new IntentFilter(Downloader.ACTION_PROGRESS));
for (String url : urls) {
DownloaderService.queue(getContext(), null, url);
}
Thread.sleep(30000);
}
}

View File

@ -442,9 +442,6 @@
<service
android:name=".net.DownloaderService"
android:exported="false" />
<service
android:name=".net.DownloadCompleteService"
android:exported="false" />
<service
android:name=".CleanCacheService"
android:exported="false" />

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

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;

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

View File

@ -1,76 +0,0 @@
package org.fdroid.fdroid.net;
import android.app.IntentService;
import android.app.NotificationManager;
import android.content.Context;
import android.content.Intent;
import android.content.pm.PackageManager;
import android.net.Uri;
import android.os.Process;
import android.support.v4.app.NotificationCompat;
import android.text.TextUtils;
import org.fdroid.fdroid.R;
import org.fdroid.fdroid.Utils;
import org.fdroid.fdroid.data.App;
import org.fdroid.fdroid.data.AppProvider;
public class DownloadCompleteService extends IntentService {
private static final String TAG = "DownloadCompleteService";
private static final String ACTION_NOTIFY = "org.fdroid.fdroid.net.action.NOTIFY";
private static final String EXTRA_PACKAGE_NAME = "org.fdroid.fdroid.net.extra.PACKAGE_NAME";
public DownloadCompleteService() {
super("DownloadCompleteService");
}
public static void notify(Context context, String packageName, String urlString) {
Intent intent = new Intent(context, DownloadCompleteService.class);
intent.setAction(ACTION_NOTIFY);
intent.setData(Uri.parse(urlString));
intent.putExtra(EXTRA_PACKAGE_NAME, packageName);
context.startService(intent);
}
@Override
protected void onHandleIntent(Intent intent) {
Process.setThreadPriority(Process.THREAD_PRIORITY_LOWEST);
if (intent != null) {
final String action = intent.getAction();
if (!ACTION_NOTIFY.equals(action)) {
Utils.debugLog(TAG, "Intent action is not ACTION_NOTIFY");
return;
}
String packageName = intent.getStringExtra(EXTRA_PACKAGE_NAME);
if (TextUtils.isEmpty(packageName)) {
Utils.debugLog(TAG, "Intent is missing EXTRA_PACKAGE_NAME");
return;
}
String title;
try {
PackageManager pm = getPackageManager();
title = String.format(getString(R.string.tap_to_update_format),
pm.getApplicationLabel(pm.getApplicationInfo(packageName, 0)));
} catch (PackageManager.NameNotFoundException e) {
App app = AppProvider.Helper.findByPackageName(getContentResolver(), packageName,
new String[]{
AppProvider.DataColumns.NAME,
});
title = String.format(getString(R.string.tap_to_install_format), app.name);
}
int requestCode = Utils.getApkUrlNotificationId(intent.getDataString());
NotificationCompat.Builder builder =
new NotificationCompat.Builder(this)
.setAutoCancel(true)
.setContentTitle(title)
.setSmallIcon(android.R.drawable.stat_sys_download_done)
.setContentIntent(DownloaderService.createAppDetailsIntent(this, requestCode, packageName))
.setContentText(getString(R.string.tap_to_install));
NotificationManager nm = (NotificationManager) getSystemService(NOTIFICATION_SERVICE);
nm.notify(Utils.getApkUrlNotificationId(intent.getDataString()), builder.build());
}
}
}

View File

@ -30,7 +30,6 @@ public abstract class Downloader {
private volatile boolean cancelled = false;
private volatile int bytesRead;
private volatile int totalBytes;
private Timer timer;
public final File outputFile;
@ -49,7 +48,7 @@ public abstract class Downloader {
/**
* For sending download progress, should only be called in {@link #progressTask}
*/
private DownloaderProgressListener downloaderProgressListener;
private volatile DownloaderProgressListener downloaderProgressListener;
protected abstract InputStream getDownloadersInputStream() throws IOException;
@ -131,9 +130,6 @@ public abstract class Downloader {
private void throwExceptionIfInterrupted() throws InterruptedException {
if (cancelled) {
Utils.debugLog(TAG, "Received interrupt, cancelling download");
if (timer != null) {
timer.cancel();
}
throw new InterruptedException();
}
}
@ -151,40 +147,44 @@ public abstract class Downloader {
* progress counter.
*/
private void copyInputToOutputStream(InputStream input, int bufferSize, OutputStream output) throws IOException, InterruptedException {
bytesRead = 0;
totalBytes = totalDownloadSize();
byte[] buffer = new byte[bufferSize];
Timer timer = new Timer();
try {
bytesRead = 0;
totalBytes = totalDownloadSize();
byte[] buffer = new byte[bufferSize];
timer = new Timer();
timer.scheduleAtFixedRate(progressTask, 0, 100);
// Getting the total download size could potentially take time, depending on how
// it is implemented, so we may as well check this before we proceed.
throwExceptionIfInterrupted();
while (true) {
int count;
if (input.available() > 0) {
int readLength = Math.min(input.available(), buffer.length);
count = input.read(buffer, 0, readLength);
} else {
count = input.read(buffer);
}
timer.scheduleAtFixedRate(progressTask, 0, 100);
// Getting the total download size could potentially take time, depending on how
// it is implemented, so we may as well check this before we proceed.
throwExceptionIfInterrupted();
if (count == -1) {
Utils.debugLog(TAG, "Finished downloading from stream");
break;
while (true) {
int count;
if (input.available() > 0) {
int readLength = Math.min(input.available(), buffer.length);
count = input.read(buffer, 0, readLength);
} else {
count = input.read(buffer);
}
throwExceptionIfInterrupted();
if (count == -1) {
Utils.debugLog(TAG, "Finished downloading from stream");
break;
}
bytesRead += count;
output.write(buffer, 0, count);
}
bytesRead += count;
output.write(buffer, 0, count);
} finally {
downloaderProgressListener = null;
timer.cancel();
timer.purge();
output.flush();
output.close();
}
timer.cancel();
timer.purge();
output.flush();
output.close();
}
/**

View File

@ -24,6 +24,7 @@ import android.app.Service;
import android.content.Context;
import android.content.Intent;
import android.content.IntentFilter;
import android.content.pm.PackageManager;
import android.net.Uri;
import android.os.Handler;
import android.os.HandlerThread;
@ -52,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
@ -60,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";
@ -94,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);
@ -133,40 +134,30 @@ 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);
}
}
@Nullable
private static String getPackageNameFromIntent(@NonNull Intent intent) {
return intent.hasExtra(EXTRA_PACKAGE_NAME) ? intent.getStringExtra(EXTRA_PACKAGE_NAME) : null;
}
private NotificationCompat.Builder createNotification(String urlString, @Nullable String packageName) {
return new NotificationCompat.Builder(this)
.setAutoCancel(true)
.setContentIntent(createAppDetailsIntent(this, 0, packageName))
.setContentIntent(createAppDetailsIntent(0, packageName))
.setContentTitle(getNotificationTitle(packageName))
.addAction(R.drawable.ic_cancel_black_24dp, getString(R.string.cancel),
createCancelDownloadIntent(this, 0, urlString))
@ -191,20 +182,20 @@ public class DownloaderService extends Service {
return getString(R.string.downloading);
}
public static PendingIntent createAppDetailsIntent(@NonNull Context context, int requestCode, @Nullable String packageName) {
private PendingIntent createAppDetailsIntent(int requestCode, String packageName) {
TaskStackBuilder stackBuilder;
if (packageName != null) {
Intent notifyIntent = new Intent(context.getApplicationContext(), AppDetails.class)
Intent notifyIntent = new Intent(getApplicationContext(), AppDetails.class)
.putExtra(AppDetails.EXTRA_APPID, packageName);
stackBuilder = TaskStackBuilder
.create(context.getApplicationContext())
.create(getApplicationContext())
.addParentStack(AppDetails.class)
.addNextIntent(notifyIntent);
} else {
Intent notifyIntent = new Intent(context.getApplicationContext(), FDroid.class);
Intent notifyIntent = new Intent(getApplicationContext(), FDroid.class);
stackBuilder = TaskStackBuilder
.create(context.getApplicationContext())
.create(getApplicationContext())
.addParentStack(FDroid.class)
.addNextIntent(notifyIntent);
}
@ -266,11 +257,11 @@ public class DownloaderService extends Service {
File downloadDir = new File(Utils.getApkCacheDir(this), uri.getHost() + "-" + uri.getPort());
downloadDir.mkdirs();
final SanitizedFile localFile = new SanitizedFile(downloadDir, uri.getLastPathSegment());
final String packageName = getPackageNameFromIntent(intent);
final String packageName = intent.getStringExtra(EXTRA_PACKAGE_NAME);
sendBroadcast(uri, Downloader.ACTION_STARTED, localFile);
if (Preferences.get().isUpdateNotificationEnabled()) {
Notification notification = createNotification(intent.getDataString(), getPackageNameFromIntent(intent)).build();
Notification notification = createNotification(intent.getDataString(), intent.getStringExtra(EXTRA_PACKAGE_NAME)).build();
startForeground(NOTIFY_DOWNLOADING, notification);
}
@ -279,26 +270,24 @@ public class DownloaderService extends Service {
downloader.setListener(new Downloader.DownloaderProgressListener() {
@Override
public void sendProgress(URL sourceUrl, int bytesRead, int totalBytes) {
if (isActive(uri.toString())) {
Intent intent = new Intent(Downloader.ACTION_PROGRESS);
intent.setData(uri);
intent.putExtra(Downloader.EXTRA_BYTES_READ, bytesRead);
intent.putExtra(Downloader.EXTRA_TOTAL_BYTES, totalBytes);
localBroadcastManager.sendBroadcast(intent);
Intent intent = new Intent(Downloader.ACTION_PROGRESS);
intent.setData(uri);
intent.putExtra(Downloader.EXTRA_BYTES_READ, bytesRead);
intent.putExtra(Downloader.EXTRA_TOTAL_BYTES, totalBytes);
localBroadcastManager.sendBroadcast(intent);
if (Preferences.get().isUpdateNotificationEnabled()) {
NotificationManager nm = (NotificationManager) getSystemService(NOTIFICATION_SERVICE);
Notification notification = createNotification(uri.toString(), packageName)
.setProgress(totalBytes, bytesRead, false)
.build();
nm.notify(NOTIFY_DOWNLOADING, notification);
}
if (Preferences.get().isUpdateNotificationEnabled()) {
NotificationManager nm = (NotificationManager) getSystemService(NOTIFICATION_SERVICE);
Notification notification = createNotification(uri.toString(), packageName)
.setProgress(totalBytes, bytesRead, false)
.build();
nm.notify(NOTIFY_DOWNLOADING, notification);
}
}
});
downloader.download();
sendBroadcast(uri, Downloader.ACTION_COMPLETE, localFile);
DownloadCompleteService.notify(this, packageName, intent.getDataString());
notifyDownloadComplete(packageName, intent.getDataString());
} catch (InterruptedException e) {
sendBroadcast(uri, Downloader.ACTION_INTERRUPTED, localFile);
} catch (IOException e) {
@ -309,14 +298,40 @@ 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;
}
/**
* Post a notification about a completed download. {@code packageName} must be a valid
* and currently in the app index database.
*/
private void notifyDownloadComplete(String packageName, String urlString) {
String title;
try {
PackageManager pm = getPackageManager();
title = String.format(getString(R.string.tap_to_update_format),
pm.getApplicationLabel(pm.getApplicationInfo(packageName, 0)));
} catch (PackageManager.NameNotFoundException e) {
App app = AppProvider.Helper.findByPackageName(getContentResolver(), packageName,
new String[]{
AppProvider.DataColumns.NAME,
});
title = String.format(getString(R.string.tap_to_install_format), app.name);
}
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(downloadUrlId, packageName))
.setContentText(getString(R.string.tap_to_install));
NotificationManager nm = (NotificationManager) getSystemService(NOTIFICATION_SERVICE);
nm.notify(downloadUrlId, builder.build());
}
private void sendBroadcast(Uri uri, String action, File file) {
sendBroadcast(uri, action, file, null);
}
@ -336,7 +351,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)
@ -357,7 +372,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)
*/
@ -373,34 +388,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());
}
/**
@ -409,7 +409,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);

View File

@ -120,7 +120,7 @@ public class HttpDownloader extends Downloader {
// workaround until NetCipher supports HTTPS SNI
// https://gitlab.com/fdroid/fdroidclient/issues/431
if (connection instanceof HttpsURLConnection
&& "f-droid.org".equals(sourceUrl.getHost())) {
&& !"f-droid.org".equals(sourceUrl.getHost())) {
((HttpsURLConnection) connection).setSSLSocketFactory(HttpsURLConnection.getDefaultSSLSocketFactory());
}