diff --git a/src/org/fdroid/fdroid/ApkDownloader.java b/src/org/fdroid/fdroid/ApkDownloader.java index 4070e0ecc..e8a58d046 100644 --- a/src/org/fdroid/fdroid/ApkDownloader.java +++ b/src/org/fdroid/fdroid/ApkDownloader.java @@ -20,27 +20,42 @@ package org.fdroid.fdroid; +import android.os.Bundle; import android.util.Log; import org.fdroid.fdroid.data.Apk; +import org.fdroid.fdroid.net.AsyncDownloadWrapper; import org.fdroid.fdroid.net.Downloader; import org.fdroid.fdroid.net.HttpDownloader; import java.io.File; +import java.io.IOException; +import java.net.MalformedURLException; +import java.security.NoSuchAlgorithmException; -public class ApkDownloader extends Thread { - private static final String TAG = "ApkDownloader"; +public class ApkDownloader implements AsyncDownloadWrapper.Listener { - public static final int EVENT_APK_DOWNLOAD_COMPLETE = 100; - public static final int EVENT_ERROR_HASH_MISMATCH = 101; - public static final int EVENT_ERROR_DOWNLOAD_FAILED = 102; - public static final int EVENT_ERROR_UNKNOWN = 103; - private Apk curapk; - private String repoaddress; - private File destdir; - private File localfile; + private static final String TAG = "org.fdroid.fdroid.ApkDownloader"; + + public static final String EVENT_APK_DOWNLOAD_COMPLETE = "apkDownloadComplete"; + public static final String EVENT_APK_DOWNLOAD_CANCELLED = "apkDownloadCancelled"; + public static final String EVENT_ERROR = "apkDownloadError"; + + public static final int ERROR_HASH_MISMATCH = 101; + public static final int ERROR_DOWNLOAD_FAILED = 102; + public static final int ERROR_UNKNOWN = 103; + + /** + * Used as a key to pass data through with an error event, explaining the type of event. + */ + public static final String EVENT_DATA_ERROR_TYPE = "apkDownloadErrorType"; + + private Apk curApk; + private String repoAddress; + private File localFile; private ProgressListener listener; + private AsyncDownloadWrapper dlWrapper = null; public void setProgressListener(ProgressListener listener) { this.listener = listener; @@ -48,84 +63,153 @@ public class ApkDownloader extends Thread { // Constructor - creates a Downloader to download the given Apk, // which must have its detail populated. - ApkDownloader(Apk apk, String repoaddress, File destdir) { - curapk = apk; - this.repoaddress = repoaddress; - this.destdir = destdir; + ApkDownloader(Apk apk, String repoAddress, File destDir) { + curApk = apk; + this.repoAddress = repoAddress; + localFile = new File(destDir, curApk.apkName); } // The downloaded APK. Valid only when getStatus() has returned STATUS.DONE. public File localFile() { - return localfile; + return localFile; } public String getRemoteAddress() { - return repoaddress + "/" + curapk.apkName.replace(" ", "%20"); + return repoAddress + "/" + curApk.apkName.replace(" ", "%20"); } - @Override - public void run() { - localfile = new File(destdir, curapk.apkName); - + private Hasher createHasher() { + Hasher hasher; try { + hasher = new Hasher(curApk.hashType, localFile); + } catch (NoSuchAlgorithmException e) { + Log.e("FDroid", "Error verifying hash of cached apk at " + localFile + ". " + + "I don't understand what the " + curApk.hashType + " hash algorithm is :("); + hasher = null; + } + return hasher; + } - // See if we already have this apk cached... - if (localfile.exists()) { - // We do - if its hash matches, we'll use it... - Hasher hash = new Hasher(curapk.hashType, localfile); - if (hash.match(curapk.hash)) { - Log.d("FDroid", "Using cached apk at " + localfile); - return; - } else { - Log.d("FDroid", "Not using cached apk at " + localfile); - localfile.delete(); - } + private boolean hashMatches() { + if (!localFile.exists()) { + return false; + } + Hasher hasher = createHasher(); + return hasher != null && hasher.match(curApk.hash); + } + + /** + * If an existing cached version exists, and matches the hash of the apk we + * want to download, then we will return true. Otherwise, we return false + * (and remove the cached file - if it exists and didn't match the correct hash). + */ + private boolean verifyOrDeleteCachedVersion() { + if (localFile.exists()) { + if (hashMatches()) { + Log.d("FDroid", "Using cached apk at " + localFile); + return true; + } else { + Log.d("FDroid", "Not using cached apk at " + localFile); + deleteLocalFile(); } + } + return false; + } - // If we haven't got the apk locally, we'll have to download it... - String remoteAddress = getRemoteAddress(); - Downloader downloader = new HttpDownloader(remoteAddress, localfile); + private void deleteLocalFile() { + if (localFile != null && localFile.exists()) { + localFile.delete(); + } + } - if (listener != null) { - downloader.setProgressListener(listener, - new ProgressListener.Event(Downloader.EVENT_PROGRESS, remoteAddress)); - } + public void download() { - Log.d(TAG, "Downloading apk from " + remoteAddress); - downloader.download(); - - if (!localfile.exists()) { - sendProgress(EVENT_ERROR_DOWNLOAD_FAILED); - return; - } - - Hasher hash = new Hasher(curapk.hashType, localfile); - if (!hash.match(curapk.hash)) { - Log.d("FDroid", "Downloaded file hash of " + hash.getHash() - + " did not match repo's " + curapk.hash); - // No point keeping a bad file, whether we're - // caching or not. - localfile.delete(); - sendProgress(EVENT_ERROR_HASH_MISMATCH); - return; - } - } catch (Exception e) { - Log.e("FDroid", "Download failed:\n" + Log.getStackTraceString(e)); - if (localfile.exists()) { - localfile.delete(); - } - sendProgress(EVENT_ERROR_UNKNOWN); + // Can we use the cached version? + if (verifyOrDeleteCachedVersion()) { + sendMessage(EVENT_APK_DOWNLOAD_COMPLETE); return; } - Log.d("FDroid", "Download finished: " + localfile); - sendProgress(EVENT_APK_DOWNLOAD_COMPLETE); - } + String remoteAddress = getRemoteAddress(); + Log.d(TAG, "Downloading apk from " + remoteAddress); - private void sendProgress(int type) { - if (listener != null) { - listener.onProgress(new ProgressListener.Event(type)); + try { + + Downloader downloader = new HttpDownloader(remoteAddress, localFile); + dlWrapper = new AsyncDownloadWrapper(downloader, this); + dlWrapper.download(); + + } catch (MalformedURLException e) { + onErrorDownloading(e.getLocalizedMessage()); + } catch (IOException e) { + onErrorDownloading(e.getLocalizedMessage()); } } -} + private void sendMessage(String type) { + sendProgressEvent(new ProgressListener.Event(type)); + } + + private void sendError(int errorType) { + Bundle data = new Bundle(1); + data.putInt(EVENT_DATA_ERROR_TYPE, errorType); + sendProgressEvent(new Event(EVENT_ERROR, data)); + } + + private void sendProgressEvent(Event event) { + if (listener != null) { + listener.onProgress(event); + } + } + + @Override + public void onReceiveTotalDownloadSize(int size) { + // Do nothing... + // Rather, we will obtain the total download size from the progress events + // when they start coming through. + } + + @Override + public void onReceiveCacheTag(String cacheTag) { + // Do nothing... + } + + @Override + public void onErrorDownloading(String localisedExceptionDetails) { + Log.e("FDroid", "Download failed: " + localisedExceptionDetails); + sendError(ERROR_DOWNLOAD_FAILED); + deleteLocalFile(); + } + + @Override + public void onDownloadComplete() { + + if (!verifyOrDeleteCachedVersion()) { + sendError(ERROR_HASH_MISMATCH); + return; + } + + Log.d("FDroid", "Download finished: " + localFile); + sendMessage(EVENT_APK_DOWNLOAD_COMPLETE); + } + + @Override + public void onDownloadCancelled() { + sendMessage(EVENT_APK_DOWNLOAD_CANCELLED); + } + + @Override + public void onProgress(Event event) { + sendProgressEvent(event); + } + + public void cancel() { + if (dlWrapper != null) { + dlWrapper.attemptCancel(); + } + } + + public Apk getApk() { + return curApk; + } +} \ No newline at end of file diff --git a/src/org/fdroid/fdroid/AppDetails.java b/src/org/fdroid/fdroid/AppDetails.java index 351a33c80..5362dc2fa 100644 --- a/src/org/fdroid/fdroid/AppDetails.java +++ b/src/org/fdroid/fdroid/AppDetails.java @@ -38,7 +38,6 @@ import android.graphics.Bitmap; import android.net.Uri; import android.os.Bundle; import android.os.Handler; -import android.os.Message; import android.preference.PreferenceManager; import android.support.v4.app.NavUtils; import android.support.v4.view.MenuItemCompat; @@ -86,7 +85,7 @@ import java.security.NoSuchAlgorithmException; import java.util.Iterator; import java.util.List; -public class AppDetails extends ListActivity { +public class AppDetails extends ListActivity implements ProgressListener { private static final String TAG = "AppDetails"; public static final int REQUEST_ENABLE_BLUETOOTH = 2; @@ -96,6 +95,7 @@ public class AppDetails extends ListActivity { private FDroidApp fdroidApp; private ApkListAdapter adapter; + private ProgressDialog progressDialog; private static class ViewHolder { TextView version; @@ -281,7 +281,7 @@ public class AppDetails extends ListActivity { private App app; private String appid; private PackageManager mPm; - private DownloadHandler downloadHandler; + private ApkDownloader downloadHandler; private boolean stateRetained; private boolean startingIgnoreAll; @@ -441,23 +441,35 @@ public class AppDetails extends ListActivity { @Override protected void onDestroy() { + // TODO: Generally need to verify the downloader stuff plays well with orientation changes... if (downloadHandler != null) { if (!stateRetained) downloadHandler.cancel(); - downloadHandler.destroy(); + removeProgressDialog(); } super.onDestroy(); } + private void removeProgressDialog() { + if (progressDialog != null) { + progressDialog.dismiss(); + progressDialog = null; + } + } + // Copy all relevant state from an old instance. This is used in // place of reset(), so it must initialize all fields normally set // there. private void copyState(AppDetails old) { + // TODO: Reimplement copyState with the new downloader stuff... But really, if we start to use fragments for + // this view, then it will probably not be relevant any more... + /* if (old.downloadHandler != null) downloadHandler = new DownloadHandler(old.downloadHandler); app = old.app; mInstalledSignature = old.mInstalledSignature; mInstalledSigID = old.mInstalledSigID; + */ } // Reset the display and list contents. Used when entering the activity, and @@ -932,9 +944,8 @@ public class AppDetails extends ListActivity { new DialogInterface.OnClickListener() { @Override public void onClick(DialogInterface dialog, - int whichButton) { - downloadHandler = new DownloadHandler( - apk, repoaddress, Utils.getApkCacheDir(getBaseContext())); + int whichButton) { + startDownload(apk, repoaddress); } }); ask_alrt.setNegativeButton(getString(R.string.no), @@ -963,8 +974,14 @@ public class AppDetails extends ListActivity { alert.show(); return; } - downloadHandler = new DownloadHandler( - apk, repoaddress, Utils.getApkCacheDir(getBaseContext())); + startDownload(apk, repoaddress); + } + + private void startDownload(Apk apk, String repoAddress) { + downloadHandler = new ApkDownloader(apk, repoAddress, Utils.getApkCacheDir(getBaseContext())); + getProgressDialog(downloadHandler.getRemoteAddress()).show(); + downloadHandler.setProgressListener(this); + downloadHandler.download(); } private void installApk(File file, String packageName) { setProgressBarIndeterminateVisibility(true); @@ -1049,123 +1066,70 @@ public class AppDetails extends ListActivity { startActivity(Intent.createChooser(shareIntent, getString(R.string.menu_share))); } - private ProgressDialog createProgressDialog(String file, int p, int max) { - final ProgressDialog pd = new ProgressDialog(this); - pd.setProgressStyle(ProgressDialog.STYLE_HORIZONTAL); - pd.setMessage(getString(R.string.download_server) + ":\n " + file); - pd.setMax(max); - pd.setProgress(p); - pd.setCancelable(true); - pd.setCanceledOnTouchOutside(false); - pd.setOnCancelListener(new DialogInterface.OnCancelListener() { - @Override - public void onCancel(DialogInterface dialog) { - downloadHandler.cancel(); - } - }); - pd.setButton(DialogInterface.BUTTON_NEUTRAL, - getString(R.string.cancel), - new DialogInterface.OnClickListener() { - @Override - public void onClick(DialogInterface dialog, int which) { - pd.cancel(); + private ProgressDialog getProgressDialog(String file) { + if (progressDialog == null) { + final ProgressDialog pd = new ProgressDialog(this); + pd.setProgressStyle(ProgressDialog.STYLE_HORIZONTAL); + pd.setMessage(getString(R.string.download_server) + ":\n " + file); + pd.setCancelable(true); + pd.setCanceledOnTouchOutside(false); + pd.setIndeterminate(true); // This will get overridden on the first progress event we receive. + pd.setOnCancelListener(new DialogInterface.OnCancelListener() { + @Override + public void onCancel(DialogInterface dialog) { + downloadHandler.cancel(); + } + }); + pd.setButton(DialogInterface.BUTTON_NEUTRAL, + getString(R.string.cancel), + new DialogInterface.OnClickListener() { + @Override + public void onClick(DialogInterface dialog, int which) { + pd.cancel(); + } } - }); - pd.show(); - return pd; + ); + progressDialog = pd; + } + return progressDialog; } - // Handler used to update the progress dialog while downloading. - private class DownloadHandler extends Handler implements ProgressListener { - private static final String TAG = "org.fdroid.fdroid.AppDetails.DownloadHandler"; - private ApkDownloader download; - private ProgressDialog pd; - private String id; + private void updateProgressDialog(int progress, int total) { + ProgressDialog pd = getProgressDialog(downloadHandler.getRemoteAddress()); + pd.setIndeterminate(false); + pd.setProgress(progress); + pd.setMax(total); + } - public DownloadHandler(Apk apk, String repoaddress, File destdir) { - id = apk.id; - download = new ApkDownloader(apk, repoaddress, destdir); - download.setProgressListener(this); - download.start(); + @Override + public void onProgress(Event event) { + boolean finished = false; + if (event.type.equals(Downloader.EVENT_PROGRESS)) { + updateProgressDialog(event.progress, event.total); + } else if (event.type.equals(ApkDownloader.EVENT_ERROR)) { + final String text; + if (event.getData().getInt(ApkDownloader.EVENT_DATA_ERROR_TYPE) == ApkDownloader.ERROR_HASH_MISMATCH) + text = getString(R.string.corrupt_download); + else + text = getString(R.string.details_notinstalled); + // this must be on the main UI thread + Toast.makeText(this, text, Toast.LENGTH_LONG).show(); + finished = true; + } else if (event.type.equals(ApkDownloader.EVENT_APK_DOWNLOAD_COMPLETE)) { + installApk(downloadHandler.localFile(), downloadHandler.getApk().id); + finished = true; + } else if (event.type.equals(ApkDownloader.EVENT_APK_DOWNLOAD_CANCELLED)) { + Toast.makeText(this, getString(R.string.download_cancelled), Toast.LENGTH_LONG).show(); + finished = true; } - public DownloadHandler(DownloadHandler oldHandler) { - if (oldHandler != null) { - download = oldHandler.download; - } - } - - private static final String MSG_EVENT_DATA = "msgEvent"; - - /** - * Subclasses must implement this to receive messages. - */ - public void handleMessage(Message msg) { - ProgressListener.Event event = msg.getData().getParcelable(MSG_EVENT_DATA); - boolean finished = false; - switch (event.type) { - case Downloader.EVENT_PROGRESS: - if (pd == null) { - pd = createProgressDialog(download.getRemoteAddress(), - event.progress, event.total); - } else { - pd.setProgress(event.progress); - } - break; - - case ApkDownloader.EVENT_ERROR_DOWNLOAD_FAILED: - case ApkDownloader.EVENT_ERROR_HASH_MISMATCH: - case ApkDownloader.EVENT_ERROR_UNKNOWN: - final String text; - if (event.type == ApkDownloader.EVENT_ERROR_HASH_MISMATCH) - text = getString(R.string.corrupt_download); - else - text = getString(R.string.details_notinstalled); - // this must be on the main UI thread - Toast.makeText(AppDetails.this, text, Toast.LENGTH_LONG).show(); - finished = true; - break; - - case ApkDownloader.EVENT_APK_DOWNLOAD_COMPLETE: - installApk(download.localFile(), id); - finished = true; - break; - } - - if (finished) { - destroy(); - } - } - - /** - * We receive events on the download thread, and then post them to - * whatever thread the DownloadHandler was run on (in our case, the UI - * thread). - * @param event - */ - public void onProgress(final ProgressListener.Event event) { - Message message = new Message(); - Bundle bundle = new Bundle(1); - bundle.putParcelable(MSG_EVENT_DATA, event); - message.setData(bundle); - sendMessage(message); - } - - public void cancel() { - // TODO: Re-implement... - } - - public void destroy() { + if (finished) { // The dialog can't be dismissed when it's not displayed, // so do it when the activity is being destroyed. - if (pd != null) { - pd.dismiss(); - pd = null; - } + removeProgressDialog(); } } - @Override protected void onActivityResult(int requestCode, int resultCode, Intent data) { // handle cases for install manager first diff --git a/src/org/fdroid/fdroid/ProgressListener.java b/src/org/fdroid/fdroid/ProgressListener.java index c7371f0ee..1b408d81d 100644 --- a/src/org/fdroid/fdroid/ProgressListener.java +++ b/src/org/fdroid/fdroid/ProgressListener.java @@ -18,7 +18,7 @@ public interface ProgressListener { public static final int NO_VALUE = Integer.MIN_VALUE; public static final String PROGRESS_DATA_REPO = "repo"; - public final int type; + public final String type; public final Bundle data; // These two are not final, so that you can create a template Event, @@ -28,22 +28,19 @@ public interface ProgressListener { public int progress; public int total; - public Event(int type) { + public Event(String type) { this(type, NO_VALUE, NO_VALUE, null); } - public Event(int type, String repoAddress) { - this(type, NO_VALUE, NO_VALUE, repoAddress); + public Event(String type, Bundle data) { + this(type, NO_VALUE, NO_VALUE, data); } - public Event(int type, int progress, int total, String repoAddress) { + public Event(String type, int progress, int total, Bundle data) { this.type = type; this.progress = progress; this.total = total; - if (TextUtils.isEmpty(repoAddress)) - this.data = new Bundle(); - else - this.data = createProgressData(repoAddress); + this.data = (data == null) ? new Bundle() : data; } @Override @@ -53,7 +50,7 @@ public interface ProgressListener { @Override public void writeToParcel(Parcel dest, int flags) { - dest.writeInt(type); + dest.writeString(type); dest.writeInt(progress); dest.writeInt(total); dest.writeBundle(data); @@ -62,8 +59,7 @@ public interface ProgressListener { public static final Parcelable.Creator CREATOR = new Parcelable.Creator() { @Override public Event createFromParcel(Parcel in) { - return new Event(in.readInt(), in.readInt(), in.readInt(), - in.readBundle().getString(PROGRESS_DATA_REPO)); + return new Event(in.readString(), in.readInt(), in.readInt(), in.readBundle()); } @Override @@ -72,16 +68,16 @@ public interface ProgressListener { } }; - public String getRepoAddress() { - return data.getString(PROGRESS_DATA_REPO); - } - - public static Bundle createProgressData(String repoAddress) { - Bundle data = new Bundle(); - data.putString(PROGRESS_DATA_REPO, repoAddress); + /** + * Can help to provide context to the listener about what process is causing the event. + * For example, the repo updater uses one listener to listen to multiple downloaders. + * When it receives an event, it doesn't know which repo download is causing the event, + * so we pass that through to the downloader when we set the progress listener. This way, + * we can ask the event for the name of the repo. + */ + public Bundle getData() { return data; } - } } diff --git a/src/org/fdroid/fdroid/RepoXMLHandler.java b/src/org/fdroid/fdroid/RepoXMLHandler.java index 541dfff2e..f85a8eafc 100644 --- a/src/org/fdroid/fdroid/RepoXMLHandler.java +++ b/src/org/fdroid/fdroid/RepoXMLHandler.java @@ -19,6 +19,7 @@ package org.fdroid.fdroid; +import android.os.Bundle; import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.App; import org.fdroid.fdroid.data.Repo; @@ -279,10 +280,12 @@ public class RepoXMLHandler extends DefaultHandler { curapp = new App(); curapp.id = attributes.getValue("", "id"); progressCounter ++; + Bundle data = new Bundle(1); + data.putString(RepoUpdater.PROGRESS_DATA_REPO_ADDRESS, repo.address); progressListener.onProgress( new ProgressListener.Event( RepoUpdater.PROGRESS_TYPE_PROCESS_XML, - progressCounter, totalAppCount, repo.address)); + progressCounter, totalAppCount, data)); } else if (localName.equals("package") && curapp != null && curapk == null) { curapk = new Apk(); diff --git a/src/org/fdroid/fdroid/UpdateService.java b/src/org/fdroid/fdroid/UpdateService.java index 06cd30a1a..bf7fd8b71 100644 --- a/src/org/fdroid/fdroid/UpdateService.java +++ b/src/org/fdroid/fdroid/UpdateService.java @@ -33,6 +33,7 @@ import android.text.TextUtils; import android.util.Log; import android.widget.Toast; import org.fdroid.fdroid.data.*; +import org.fdroid.fdroid.net.Downloader; import org.fdroid.fdroid.updater.RepoUpdater; import java.util.*; @@ -47,6 +48,14 @@ public class UpdateService extends IntentService implements ProgressListener { public static final int STATUS_ERROR = 2; public static final int STATUS_INFO = 3; + // I don't like that I've had to dupliacte the statuses above with strings here, however + // one method of communication/notification is using ResultReceiver (int status codes) + // while the other uses progress events (string event types). + public static final String EVENT_COMPLETE_WITH_CHANGES = "repoUpdateComplete (changed)"; + public static final String EVENT_COMPLETE_AND_SAME = "repoUpdateComplete (not changed)"; + public static final String EVENT_ERROR = "repoUpdateError"; + public static final String EVENT_INFO = "repoUpdateInfo"; + public static final String EXTRA_RECEIVER = "receiver"; public static final String EXTRA_ADDRESS = "address"; @@ -97,28 +106,31 @@ public class UpdateService extends IntentService implements ProgressListener { return this; } + private void forwardEvent(String type) { + if (listener != null) { + listener.onProgress(new Event(type)); + } + } + @Override protected void onReceiveResult(int resultCode, Bundle resultData) { String message = resultData.getString(UpdateService.RESULT_MESSAGE); boolean finished = false; if (resultCode == UpdateService.STATUS_ERROR) { + forwardEvent(EVENT_ERROR); Toast.makeText(context, message, Toast.LENGTH_LONG).show(); finished = true; - } else if (resultCode == UpdateService.STATUS_COMPLETE_WITH_CHANGES - || resultCode == UpdateService.STATUS_COMPLETE_AND_SAME) { + } else if (resultCode == UpdateService.STATUS_COMPLETE_WITH_CHANGES) { + forwardEvent(EVENT_COMPLETE_WITH_CHANGES); + finished = true; + } else if (resultCode == UpdateService.STATUS_COMPLETE_AND_SAME) { + forwardEvent(EVENT_COMPLETE_AND_SAME); finished = true; } else if (resultCode == UpdateService.STATUS_INFO) { + forwardEvent(EVENT_INFO); dialog.setMessage(message); } - // Forward the progress event on to anybody else who'd like to know. - if (listener != null) { - Parcelable event = resultData.getParcelable(UpdateService.RESULT_EVENT); - if (event != null && event instanceof Event) { - listener.onProgress((Event)event); - } - } - if (finished && dialog.isShowing()) try { dialog.dismiss(); @@ -185,17 +197,10 @@ public class UpdateService extends IntentService implements ProgressListener { } protected void sendStatus(int statusCode, String message) { - sendStatus(statusCode, message, null); - } - - protected void sendStatus(int statusCode, String message, Event event) { if (receiver != null) { Bundle resultData = new Bundle(); if (message != null && message.length() > 0) resultData.putString(RESULT_MESSAGE, message); - if (event == null) - event = new ProgressListener.Event(statusCode); - resultData.putParcelable(RESULT_EVENT, event); receiver.send(statusCode, resultData); } } @@ -675,13 +680,15 @@ public class UpdateService extends IntentService implements ProgressListener { @Override public void onProgress(ProgressListener.Event event) { String message = ""; - String repoAddress = event.getRepoAddress(); - if (event.type == RepoUpdater.PROGRESS_TYPE_DOWNLOAD) { - String downloadedSize = Utils.getFriendlySize( event.progress ); - String totalSize = Utils.getFriendlySize( event.total ); + // TODO: Switch to passing through Bundles of data with the event, rather than a repo address. They are + // now much more general purpose then just repo downloading. + String repoAddress = event.getData().getString(RepoUpdater.PROGRESS_DATA_REPO_ADDRESS); + if (event.type.equals(Downloader.EVENT_PROGRESS)) { + String downloadedSize = Utils.getFriendlySize(event.progress); + String totalSize = Utils.getFriendlySize(event.total); int percent = (int)((double)event.progress/event.total * 100); message = getString(R.string.status_download, repoAddress, downloadedSize, totalSize, percent); - } else if (event.type == RepoUpdater.PROGRESS_TYPE_PROCESS_XML) { + } else if (event.type.equals(RepoUpdater.PROGRESS_TYPE_PROCESS_XML)) { message = getString(R.string.status_processing_xml, repoAddress, event.progress, event.total); } sendStatus(STATUS_INFO, message); diff --git a/src/org/fdroid/fdroid/net/AsyncDownloadWrapper.java b/src/org/fdroid/fdroid/net/AsyncDownloadWrapper.java new file mode 100644 index 000000000..01aba30cb --- /dev/null +++ b/src/org/fdroid/fdroid/net/AsyncDownloadWrapper.java @@ -0,0 +1,133 @@ +package org.fdroid.fdroid.net; + +import android.os.Bundle; +import android.os.Handler; +import android.os.Message; +import android.util.Log; +import org.fdroid.fdroid.ProgressListener; + +import java.io.IOException; + +public class AsyncDownloadWrapper extends Handler { + + private static final String TAG = "org.fdroid.fdroid.net.AsyncDownloadWrapper"; + + private static final int MSG_PROGRESS = 1; + private static final int MSG_DOWNLOAD_COMPLETE = 2; + private static final int MSG_DOWNLOAD_CANCELLED = 3; + private static final int MSG_ERROR = 4; + private static final String MSG_DATA = "data"; + + private Downloader downloader; + private Listener listener; + private DownloadThread downloadThread = null; + + /** + * Normally the listener would be provided using a setListener method. + * However for the purposes of this async downloader, it doesn't make + * sense to have an async task without any way to notify the outside + * world about completion. Therefore, we require the listener as a + * parameter to the constructor. + */ + public AsyncDownloadWrapper(Downloader downloader, Listener listener) { + this.downloader = downloader; + this.listener = listener; + } + + public void fetchTotalDownloadSize() { + int size = downloader.totalDownloadSize(); + listener.onReceiveTotalDownloadSize(size); + } + + public void fetchCacheTag() { + String cacheTag = downloader.getCacheTag(); + listener.onReceiveCacheTag(cacheTag); + } + + public void download() { + downloadThread = new DownloadThread(); + downloadThread.start(); + } + + public void attemptCancel() { + if (downloadThread != null) { + downloadThread.interrupt(); + } + } + + public static class NotDownloadingException extends Exception { + public NotDownloadingException(String message) { + super(message); + } + } + + public void cancelDownload() throws NotDownloadingException { + if (downloadThread == null) { + throw new RuntimeException("Can't cancel download, it hasn't started yet."); + } else if (!downloadThread.isAlive()) { + throw new RuntimeException("Can't cancel download, it is already finished."); + } + + downloadThread.interrupt(); + } + + public void handleMessage(Message message) { + if (message.arg1 == MSG_PROGRESS) { + Bundle data = message.getData(); + ProgressListener.Event event = data.getParcelable(MSG_DATA); + listener.onProgress(event); + } else if (message.arg1 == MSG_DOWNLOAD_COMPLETE) { + listener.onDownloadComplete(); + } else if (message.arg1 == MSG_DOWNLOAD_CANCELLED) { + listener.onDownloadCancelled(); + } else if (message.arg1 == MSG_ERROR) { + listener.onErrorDownloading(message.getData().getString(MSG_DATA)); + } + } + + public interface Listener extends ProgressListener { + public void onReceiveTotalDownloadSize(int size); + public void onReceiveCacheTag(String cacheTag); + public void onErrorDownloading(String localisedExceptionDetails); + public void onDownloadComplete(); + public void onDownloadCancelled(); + } + + private class DownloadThread extends Thread implements ProgressListener { + + public void run() { + try { + downloader.setProgressListener(this); + downloader.download(); + sendMessage(MSG_DOWNLOAD_COMPLETE); + } catch (InterruptedException e) { + sendMessage(MSG_DOWNLOAD_CANCELLED); + } catch (IOException e) { + Log.e(TAG, e.getMessage() + ": " + Log.getStackTraceString(e)); + Bundle data = new Bundle(1); + data.putString(MSG_DATA, e.getLocalizedMessage()); + Message message = new Message(); + message.arg1 = MSG_ERROR; + message.setData(data); + AsyncDownloadWrapper.this.sendMessage(message); + } + } + + private void sendMessage(int messageType) { + Message message = new Message(); + message.arg1 = messageType; + AsyncDownloadWrapper.this.sendMessage(message); + } + + @Override + public void onProgress(Event event) { + Message message = new Message(); + Bundle data = new Bundle(); + data.putParcelable(MSG_DATA, event); + message.setData(data); + message.arg1 = MSG_PROGRESS; + AsyncDownloadWrapper.this.sendMessage(message); + } + } + +} diff --git a/src/org/fdroid/fdroid/net/Downloader.java b/src/org/fdroid/fdroid/net/Downloader.java index c26ef7eb2..8f28a252a 100644 --- a/src/org/fdroid/fdroid/net/Downloader.java +++ b/src/org/fdroid/fdroid/net/Downloader.java @@ -1,6 +1,7 @@ package org.fdroid.fdroid.net; import android.content.Context; +import android.os.Bundle; import android.util.Log; import org.fdroid.fdroid.ProgressListener; @@ -15,16 +16,16 @@ import java.io.OutputStream; import java.net.MalformedURLException; public abstract class Downloader { + private static final String TAG = "org.fdroid.fdroid.net.Downloader"; - private OutputStream outputStream; - private ProgressListener progressListener = null; - private ProgressListener.Event progressEvent = null; - private File outputFile; + private ProgressListener progressListener = null; + private Bundle eventData = null; + private File outputFile; protected String cacheTag = null; - public static final int EVENT_PROGRESS = 1; + public static final String EVENT_PROGRESS = "downloadProgress"; public abstract InputStream inputStream() throws IOException; @@ -52,6 +53,7 @@ public abstract class Downloader { * @see org.fdroid.fdroid.net.Downloader#getFile() */ public Downloader(File destFile, Context ctx) throws IOException { + // TODO: Reimplement (is it still necessary? In what context was it being used before?) } public Downloader(OutputStream output) @@ -60,11 +62,13 @@ public abstract class Downloader { outputFile = null; } - public void setProgressListener(ProgressListener progressListener, - ProgressListener.Event progressEvent) { - Log.i(TAG, "setProgressListener(ProgressListener listener, ProgressListener.Event progressEvent)"); - this.progressListener = progressListener; - this.progressEvent = progressEvent; + public void setProgressListener(ProgressListener listener) { + setProgressListener(listener, null); + } + + public void setProgressListener(ProgressListener listener, Bundle eventData) { + this.progressListener = listener; + this.eventData = eventData; } /** @@ -101,29 +105,75 @@ public abstract class Downloader { public abstract int totalDownloadSize(); - private void setupProgressListener() { - Log.i(TAG, "setupProgressListener"); - if (progressListener != null && progressEvent != null) { - progressEvent.total = totalDownloadSize(); - } + /** + * Helper function for synchronous downloads (i.e. those *not* using AsyncDownloadWrapper), + * which don't really want to bother dealing with an InterruptedException. + * The InterruptedException thrown from download() is there to enable cancelling asynchronous + * downloads, but regular synchronous downloads cannot be cancelled because download() will + * block until completed. + * @throws IOException + */ + public void downloadUninterrupted() throws IOException { + try { + download(); + } catch (InterruptedException ignored) {} } - public abstract void download() throws IOException; + public abstract void download() throws IOException, InterruptedException; public abstract boolean isCached(); - protected void downloadFromStream() throws IOException { + protected void downloadFromStream() throws IOException, InterruptedException { Log.d(TAG, "Downloading from stream"); - setupProgressListener(); InputStream input = null; try { input = inputStream(); - Utils.copy(input, outputStream, - progressListener, progressEvent); + copyInputToOutputStream(inputStream()); } finally { Utils.closeQuietly(outputStream); Utils.closeQuietly(input); } } + protected void copyInputToOutputStream(InputStream input) throws IOException, InterruptedException { + + byte[] buffer = new byte[Utils.BUFFER_SIZE]; + int bytesRead = 0; + int totalBytes = totalDownloadSize(); + sendProgress(bytesRead, totalBytes); + while (true) { + + // In a synchronous download (the usual usage of the Downloader interface), + // you will not be able to interrupt this because the thread will block + // after you have called download(). However if you use the AsyncDownloadWrapper, + // then it will use this mechanism to cancel the download. + if (Thread.interrupted()) { + // TODO: Do we need to provide more information to whoever needs it, + // so they can, for example, remove any partially created files? + Log.d(TAG, "Received interrupt, cancelling download"); + throw new InterruptedException(); + } + + int count = input.read(buffer); + bytesRead += count; + sendProgress(bytesRead, totalBytes); + if (count == -1) { + Log.d(TAG, "Finished downloading from stream"); + break; + } + outputStream.write(buffer, 0, count); + } + outputStream.flush(); + } + + protected void sendProgress(int bytesRead, int totalBytes) { + sendProgress(new ProgressListener.Event(EVENT_PROGRESS, bytesRead, totalBytes, eventData)); + } + + protected void sendProgress(ProgressListener.Event event) { + if (progressListener != null) { + progressListener.onProgress(event); + } + } + } diff --git a/src/org/fdroid/fdroid/net/HttpDownloader.java b/src/org/fdroid/fdroid/net/HttpDownloader.java index 3f7920716..177fa6de1 100644 --- a/src/org/fdroid/fdroid/net/HttpDownloader.java +++ b/src/org/fdroid/fdroid/net/HttpDownloader.java @@ -66,7 +66,7 @@ public class HttpDownloader extends Downloader { // empty) may contain an etag value for the response, or it may be left // empty if none was available. @Override - public void download() throws IOException { + public void download() throws IOException, InterruptedException { try { connection = (HttpURLConnection)sourceUrl.openConnection(); @@ -122,7 +122,7 @@ public class HttpDownloader extends Downloader { @Override public boolean hasChanged() { - return this.statusCode == 200; + return this.statusCode != 304; } } diff --git a/src/org/fdroid/fdroid/updater/RepoUpdater.java b/src/org/fdroid/fdroid/updater/RepoUpdater.java index 6f1ce3164..40864f881 100644 --- a/src/org/fdroid/fdroid/updater/RepoUpdater.java +++ b/src/org/fdroid/fdroid/updater/RepoUpdater.java @@ -2,6 +2,7 @@ package org.fdroid.fdroid.updater; import android.content.ContentValues; import android.content.Context; +import android.os.Bundle; import android.util.Log; import org.fdroid.fdroid.ProgressListener; @@ -32,8 +33,9 @@ import javax.xml.parsers.SAXParserFactory; abstract public class RepoUpdater { - public static final int PROGRESS_TYPE_DOWNLOAD = 1; - public static final int PROGRESS_TYPE_PROCESS_XML = 2; + public static final String PROGRESS_TYPE_PROCESS_XML = "processingXml"; + + public static final String PROGRESS_DATA_REPO_ADDRESS = "repoAddress"; public static RepoUpdater createUpdaterFor(Context ctx, Repo repo) { if (repo.fingerprint == null && repo.pubkey == null) { @@ -91,11 +93,12 @@ abstract public class RepoUpdater { downloader.setCacheTag(repo.lastetag); if (progressListener != null) { // interactive session, show progress - downloader.setProgressListener(progressListener, - new ProgressListener.Event(PROGRESS_TYPE_DOWNLOAD, repo.address)); + Bundle data = new Bundle(1); + data.putString(PROGRESS_DATA_REPO_ADDRESS, getIndexAddress()); + downloader.setProgressListener(progressListener, data); } - downloader.download(); + downloader.downloadUninterrupted(); if (downloader.isCached()) { // The index is unchanged since we last read it. We just mark diff --git a/src/org/fdroid/fdroid/views/fragments/RepoDetailsFragment.java b/src/org/fdroid/fdroid/views/fragments/RepoDetailsFragment.java index 0a3ff5d2f..471a6a654 100644 --- a/src/org/fdroid/fdroid/views/fragments/RepoDetailsFragment.java +++ b/src/org/fdroid/fdroid/views/fragments/RepoDetailsFragment.java @@ -203,7 +203,7 @@ public class RepoDetailsFragment extends Fragment { UpdateService.updateRepoNow(repo.address, getActivity()).setListener(new ProgressListener() { @Override public void onProgress(Event event) { - if (event.type == UpdateService.STATUS_COMPLETE_WITH_CHANGES) { + if (event.type.equals(UpdateService.EVENT_COMPLETE_WITH_CHANGES)) { repo = loadRepoDetails(); updateView((ViewGroup)getView()); } diff --git a/src/org/fdroid/fdroid/views/fragments/RepoListFragment.java b/src/org/fdroid/fdroid/views/fragments/RepoListFragment.java index d5b23c5ac..f98a18834 100644 --- a/src/org/fdroid/fdroid/views/fragments/RepoListFragment.java +++ b/src/org/fdroid/fdroid/views/fragments/RepoListFragment.java @@ -219,8 +219,8 @@ public class RepoListFragment extends ListFragment UpdateService.updateNow(getActivity()).setListener(new ProgressListener() { @Override public void onProgress(Event event) { - if (event.type == UpdateService.STATUS_COMPLETE_AND_SAME || - event.type == UpdateService.STATUS_COMPLETE_WITH_CHANGES) { + if (event.type.equals(UpdateService.EVENT_COMPLETE_AND_SAME) || + event.type.equals(UpdateService.EVENT_COMPLETE_WITH_CHANGES)) { // No need to prompt to update any more, we just did it! changed = false; }