diff --git a/src/org/fdroid/fdroid/AppDetails.java b/src/org/fdroid/fdroid/AppDetails.java index 2c117d850..351a33c80 100644 --- a/src/org/fdroid/fdroid/AppDetails.java +++ b/src/org/fdroid/fdroid/AppDetails.java @@ -38,6 +38,7 @@ 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; @@ -932,8 +933,8 @@ public class AppDetails extends ListActivity { @Override public void onClick(DialogInterface dialog, int whichButton) { - downloadHandler = new DownloadHandler(activity, apk, - repoaddress, Utils.getApkCacheDir(getBaseContext())); + downloadHandler = new DownloadHandler( + apk, repoaddress, Utils.getApkCacheDir(getBaseContext())); } }); ask_alrt.setNegativeButton(getString(R.string.no), @@ -962,8 +963,8 @@ public class AppDetails extends ListActivity { alert.show(); return; } - downloadHandler = new DownloadHandler(activity, apk, repoaddress, - Utils.getApkCacheDir(getBaseContext())); + downloadHandler = new DownloadHandler( + apk, repoaddress, Utils.getApkCacheDir(getBaseContext())); } private void installApk(File file, String packageName) { setProgressBarIndeterminateVisibility(true); @@ -1076,14 +1077,12 @@ public class AppDetails extends ListActivity { // Handler used to update the progress dialog while downloading. private class DownloadHandler extends Handler implements ProgressListener { - private static final String TAG = "AppDetails.DownloadHandler"; - private Activity activity; + private static final String TAG = "org.fdroid.fdroid.AppDetails.DownloadHandler"; private ApkDownloader download; private ProgressDialog pd; private String id; - public DownloadHandler(Activity activity, Apk apk, String repoaddress, File destdir) { - this.activity = activity; + public DownloadHandler(Apk apk, String repoaddress, File destdir) { id = apk.id; download = new ApkDownloader(apk, repoaddress, destdir); download.setProgressListener(this); @@ -1096,23 +1095,22 @@ public class AppDetails extends ListActivity { } } - public void onProgress(final ProgressListener.Event event) { + 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: - activity.runOnUiThread(new Runnable() { - - @Override - public void run() { - // this must be on the main UI thread - if (pd == null) { - pd = createProgressDialog(download.getRemoteAddress(), - event.progress, event.total); - } else { - pd.setProgress(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: @@ -1123,14 +1121,8 @@ public class AppDetails extends ListActivity { text = getString(R.string.corrupt_download); else text = getString(R.string.details_notinstalled); - activity.runOnUiThread(new Runnable() { - - @Override - public void run() { - // this must be on the main UI thread - Toast.makeText(AppDetails.this, text, Toast.LENGTH_LONG).show(); - } - }); + // this must be on the main UI thread + Toast.makeText(AppDetails.this, text, Toast.LENGTH_LONG).show(); finished = true; break; @@ -1138,7 +1130,6 @@ public class AppDetails extends ListActivity { installApk(download.localFile(), id); finished = true; break; - } if (finished) { @@ -1146,6 +1137,20 @@ public class AppDetails extends ListActivity { } } + /** + * 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... } diff --git a/src/org/fdroid/fdroid/net/Downloader.java b/src/org/fdroid/fdroid/net/Downloader.java index 61b600ab0..c26ef7eb2 100644 --- a/src/org/fdroid/fdroid/net/Downloader.java +++ b/src/org/fdroid/fdroid/net/Downloader.java @@ -15,14 +15,14 @@ import java.io.OutputStream; import java.net.MalformedURLException; public abstract class Downloader { - private static final String TAG = "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; - protected String eTag = null; + protected String cacheTag = null; public static final int EVENT_PROGRESS = 1; @@ -68,20 +68,24 @@ public abstract class Downloader { } /** - * If you ask for the eTag before calling download(), you will get the + * If you ask for the cacheTag before calling download(), you will get the * same one you passed in (if any). If you call it after download(), you - * will get the new eTag from the server, or null if there was none. + * will get the new cacheTag from the server, or null if there was none. */ - public String getETag() { - return eTag; + public String getCacheTag() { + return cacheTag; } /** - * If this eTag matches that returned by the server, then no download will + * If this cacheTag matches that returned by the server, then no download will * take place, and a status code of 304 will be returned by download(). */ - public void setETag(String eTag) { - this.eTag = eTag; + public void setCacheTag(String cacheTag) { + this.cacheTag = cacheTag; + } + + protected boolean wantToCheckCache() { + return cacheTag != null; } /** @@ -104,8 +108,12 @@ public abstract class Downloader { } } - public void download() throws IOException { - Log.i(TAG, "download"); + public abstract void download() throws IOException; + + public abstract boolean isCached(); + + protected void downloadFromStream() throws IOException { + Log.d(TAG, "Downloading from stream"); setupProgressListener(); InputStream input = null; try { diff --git a/src/org/fdroid/fdroid/net/HttpDownloader.java b/src/org/fdroid/fdroid/net/HttpDownloader.java index 6fe79e7a7..3f7920716 100644 --- a/src/org/fdroid/fdroid/net/HttpDownloader.java +++ b/src/org/fdroid/fdroid/net/HttpDownloader.java @@ -11,11 +11,12 @@ import java.io.OutputStream; import java.net.HttpURLConnection; import java.net.MalformedURLException; import java.net.URL; +import java.net.UnknownHostException; import javax.net.ssl.SSLHandshakeException; public class HttpDownloader extends Downloader { - private static final String TAG = "HttpDownloader"; + private static final String TAG = "org.fdroid.fdroid.net.HttpDownloader"; private static final String HEADER_IF_NONE_MATCH = "If-None-Match"; private static final String HEADER_FIELD_ETAG = "ETag"; @@ -68,29 +69,22 @@ public class HttpDownloader extends Downloader { public void download() throws IOException { try { connection = (HttpURLConnection)sourceUrl.openConnection(); - setupCacheCheck(); - statusCode = connection.getResponseCode(); - Log.i(TAG, "download " + statusCode); - if (statusCode == 304) { - // The index is unchanged since we last read it. We just mark - // everything that came from this repo as being updated. - Log.d("FDroid", "Repo index for " + sourceUrl - + " is up to date (by etag)"); - } else if (statusCode == 200) { - download(); - updateCacheCheck(); + + if (wantToCheckCache()) { + setupCacheCheck(); + Log.i(TAG, "Checking cached status of " + sourceUrl); + statusCode = connection.getResponseCode(); + } + + if (isCached()) { + Log.i(TAG, sourceUrl + " is cached, so not downloading (HTTP " + statusCode + ")"); } else { - // Is there any code other than 200 which still returns - // content? Just in case, lets try to clean up. - if (getFile() != null) { - getFile().delete(); - } - throw new IOException( - "Failed to update repo " + sourceUrl + - " - HTTP response " + statusCode); + Log.i(TAG, "Downloading from " + sourceUrl); + downloadFromStream(); + updateCacheCheck(); } } catch (SSLHandshakeException e) { - // TODO this should be handled better + // TODO this should be handled better, it is not internationalised here. throw new IOException( "A problem occurred while establishing an SSL " + "connection. If this problem persists, AND you have a " + @@ -99,14 +93,18 @@ public class HttpDownloader extends Downloader { } } + public boolean isCached() { + return wantToCheckCache() && statusCode == 304; + } + private void setupCacheCheck() { - if (eTag != null) { - connection.setRequestProperty(HEADER_IF_NONE_MATCH, eTag); + if (cacheTag != null) { + connection.setRequestProperty(HEADER_IF_NONE_MATCH, cacheTag); } } private void updateCacheCheck() { - eTag = connection.getHeaderField(HEADER_FIELD_ETAG); + cacheTag = connection.getHeaderField(HEADER_FIELD_ETAG); } // Testing in the emulator for me, showed that figuring out the diff --git a/src/org/fdroid/fdroid/updater/RepoUpdater.java b/src/org/fdroid/fdroid/updater/RepoUpdater.java index 2186d50a1..6f1ce3164 100644 --- a/src/org/fdroid/fdroid/updater/RepoUpdater.java +++ b/src/org/fdroid/fdroid/updater/RepoUpdater.java @@ -26,7 +26,6 @@ import java.util.ArrayList; import java.util.Date; import java.util.List; -import javax.net.ssl.SSLHandshakeException; import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; @@ -89,7 +88,7 @@ abstract public class RepoUpdater { Downloader downloader = null; try { downloader = new HttpDownloader(getIndexAddress(), context); - downloader.setETag(repo.lastetag); + downloader.setCacheTag(repo.lastetag); if (progressListener != null) { // interactive session, show progress downloader.setProgressListener(progressListener, @@ -97,6 +96,14 @@ abstract public class RepoUpdater { } downloader.download(); + + if (downloader.isCached()) { + // The index is unchanged since we last read it. We just mark + // everything that came from this repo as being updated. + Log.d("FDroid", "Repo index for " + getIndexAddress() + + " is up to date (by etag)"); + } + } catch (IOException e) { if (downloader != null && downloader.getFile() != null) { downloader.getFile().delete(); @@ -160,7 +167,7 @@ abstract public class RepoUpdater { reader.parse(is); apps = handler.getApps(); apks = handler.getApks(); - updateRepo(handler, downloader.getETag()); + updateRepo(handler, downloader.getCacheTag()); } } catch (SAXException e) { throw new UpdateException(