From 91f2f1014efa2928cf278b647f5492384b7bae30 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 25 Feb 2014 21:40:10 -0500 Subject: [PATCH] SKETCH - DOES NOT WORK: move HTTP code into HttpDownloader This is the beginning of moving to a Downloader class and subclasses that are used everywhere for downloading APKs, indexes, and icons. I think that the ETag value can probably be used everywhere as the unique ID for testing whether to download content, so I moved that to the super class Downloader. But that's just a guess as of now... --- src/org/fdroid/fdroid/ApkDownloader.java | 11 +-- src/org/fdroid/fdroid/net/Downloader.java | 48 +++++++++-- src/org/fdroid/fdroid/net/HttpDownloader.java | 84 ++++++++++--------- .../fdroid/fdroid/updater/RepoUpdater.java | 50 ++++------- 4 files changed, 108 insertions(+), 85 deletions(-) diff --git a/src/org/fdroid/fdroid/ApkDownloader.java b/src/org/fdroid/fdroid/ApkDownloader.java index 006148fa5..4070e0ecc 100644 --- a/src/org/fdroid/fdroid/ApkDownloader.java +++ b/src/org/fdroid/fdroid/ApkDownloader.java @@ -20,13 +20,14 @@ package org.fdroid.fdroid; -import java.io.*; - import android.util.Log; + import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.net.Downloader; import org.fdroid.fdroid.net.HttpDownloader; +import java.io.File; + public class ApkDownloader extends Thread { private static final String TAG = "ApkDownloader"; @@ -83,7 +84,7 @@ public class ApkDownloader extends Thread { // If we haven't got the apk locally, we'll have to download it... String remoteAddress = getRemoteAddress(); - HttpDownloader downloader = new HttpDownloader(remoteAddress, localfile); + Downloader downloader = new HttpDownloader(remoteAddress, localfile); if (listener != null) { downloader.setProgressListener(listener, @@ -91,9 +92,9 @@ public class ApkDownloader extends Thread { } Log.d(TAG, "Downloading apk from " + remoteAddress); - int httpStatus = downloader.downloadHttpFile(); + downloader.download(); - if (httpStatus != 200 || !localfile.exists()) { + if (!localfile.exists()) { sendProgress(EVENT_ERROR_DOWNLOAD_FAILED); return; } diff --git a/src/org/fdroid/fdroid/net/Downloader.java b/src/org/fdroid/fdroid/net/Downloader.java index fac02f64c..61b600ab0 100644 --- a/src/org/fdroid/fdroid/net/Downloader.java +++ b/src/org/fdroid/fdroid/net/Downloader.java @@ -1,17 +1,29 @@ package org.fdroid.fdroid.net; -import java.io.*; -import java.net.*; -import android.content.*; -import org.fdroid.fdroid.*; +import android.content.Context; +import android.util.Log; + +import org.fdroid.fdroid.ProgressListener; +import org.fdroid.fdroid.Utils; + +import java.io.File; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.net.MalformedURLException; public abstract class Downloader { + private static final String TAG = "Downloader"; private OutputStream outputStream; private ProgressListener progressListener = null; private ProgressListener.Event progressEvent = null; private File outputFile; + protected String eTag = null; + public static final int EVENT_PROGRESS = 1; public abstract InputStream inputStream() throws IOException; @@ -50,10 +62,28 @@ public abstract class Downloader { public void setProgressListener(ProgressListener progressListener, ProgressListener.Event progressEvent) { + Log.i(TAG, "setProgressListener(ProgressListener listener, ProgressListener.Event progressEvent)"); this.progressListener = progressListener; this.progressEvent = progressEvent; } + /** + * If you ask for the eTag 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. + */ + public String getETag() { + return eTag; + } + + /** + * If this eTag 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; + } + /** * Only available if you passed a context object into the constructor * (rather than an outputStream, which may or may not be associated with @@ -63,15 +93,19 @@ public abstract class Downloader { return outputFile; } + public abstract boolean hasChanged(); + + public abstract int totalDownloadSize(); + private void setupProgressListener() { + Log.i(TAG, "setupProgressListener"); if (progressListener != null && progressEvent != null) { progressEvent.total = totalDownloadSize(); } } - protected abstract int totalDownloadSize(); - - protected void download() throws IOException { + public void download() throws IOException { + Log.i(TAG, "download"); setupProgressListener(); InputStream input = null; try { diff --git a/src/org/fdroid/fdroid/net/HttpDownloader.java b/src/org/fdroid/fdroid/net/HttpDownloader.java index 74347ae0b..6fe79e7a7 100644 --- a/src/org/fdroid/fdroid/net/HttpDownloader.java +++ b/src/org/fdroid/fdroid/net/HttpDownloader.java @@ -1,19 +1,26 @@ package org.fdroid.fdroid.net; import android.content.Context; +import android.util.Log; -import java.io.*; +import java.io.File; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; import java.net.HttpURLConnection; import java.net.MalformedURLException; import java.net.URL; +import javax.net.ssl.SSLHandshakeException; + public class HttpDownloader extends Downloader { + private static final String TAG = "HttpDownloader"; private static final String HEADER_IF_NONE_MATCH = "If-None-Match"; private static final String HEADER_FIELD_ETAG = "ETag"; private URL sourceUrl; - private String eTag = null; private HttpURLConnection connection; private int statusCode = -1; @@ -57,49 +64,48 @@ public class HttpDownloader extends Downloader { // In the event of a 200 response ONLY, 'retag' (which should be passed // empty) may contain an etag value for the response, or it may be left // empty if none was available. - public int downloadHttpFile() throws IOException { - connection = (HttpURLConnection)sourceUrl.openConnection(); - setupCacheCheck(); - statusCode = connection.getResponseCode(); - if (statusCode == 200) { - download(); - updateCacheCheck(); + @Override + 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(); + } 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); + } + } catch (SSLHandshakeException e) { + // TODO this should be handled better + throw new IOException( + "A problem occurred while establishing an SSL " + + "connection. If this problem persists, AND you have a " + + "very old device, you could try using http instead of " + + "https for the repo URL." + Log.getStackTraceString(e) ); } - return statusCode; } - /** - * Only available after downloading a file. - */ - public int getStatusCode() { - return statusCode; - } - - /** - * If you ask for the eTag 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. - */ - public String getETag() { - return eTag; - } - - /** - * If this eTag 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; - } - - - protected void setupCacheCheck() { + private void setupCacheCheck() { if (eTag != null) { connection.setRequestProperty(HEADER_IF_NONE_MATCH, eTag); } } - protected void updateCacheCheck() { + private void updateCacheCheck() { eTag = connection.getHeaderField(HEADER_FIELD_ETAG); } @@ -111,10 +117,12 @@ public class HttpDownloader extends Downloader { // on my connection. I think the 1/1.5 seconds is worth it, // because as the repo grows, the tradeoff will // become more worth it. - protected int totalDownloadSize() { + @Override + public int totalDownloadSize() { return connection.getContentLength(); } + @Override public boolean hasChanged() { return this.statusCode == 200; } diff --git a/src/org/fdroid/fdroid/updater/RepoUpdater.java b/src/org/fdroid/fdroid/updater/RepoUpdater.java index 357b75dfa..2186d50a1 100644 --- a/src/org/fdroid/fdroid/updater/RepoUpdater.java +++ b/src/org/fdroid/fdroid/updater/RepoUpdater.java @@ -3,6 +3,7 @@ package org.fdroid.fdroid.updater; import android.content.ContentValues; import android.content.Context; import android.util.Log; + import org.fdroid.fdroid.ProgressListener; import org.fdroid.fdroid.RepoXMLHandler; import org.fdroid.fdroid.Utils; @@ -10,19 +11,25 @@ import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.App; import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.data.RepoProvider; +import org.fdroid.fdroid.net.Downloader; import org.fdroid.fdroid.net.HttpDownloader; import org.xml.sax.InputSource; import org.xml.sax.SAXException; import org.xml.sax.XMLReader; +import java.io.BufferedReader; +import java.io.File; +import java.io.FileNotFoundException; +import java.io.FileReader; +import java.io.IOException; +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; -import java.io.*; -import java.util.ArrayList; -import java.util.Date; -import java.util.List; abstract public class RepoUpdater { @@ -78,8 +85,8 @@ abstract public class RepoUpdater { protected abstract String getIndexAddress(); - protected HttpDownloader downloadIndex() throws UpdateException { - HttpDownloader downloader = null; + protected Downloader downloadIndex() throws UpdateException { + Downloader downloader = null; try { downloader = new HttpDownloader(getIndexAddress(), context); downloader.setETag(repo.lastetag); @@ -89,34 +96,7 @@ abstract public class RepoUpdater { new ProgressListener.Event(PROGRESS_TYPE_DOWNLOAD, repo.address)); } - int status = downloader.downloadHttpFile(); - - if (status == 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 " + repo.address - + " is up to date (by etag)"); - } else if (status == 200) { - // Nothing needed to be done here... - } else { - // Is there any code other than 200 which still returns - // content? Just in case, lets try to clean up. - if (downloader.getFile() != null) { - downloader.getFile().delete(); - } - throw new UpdateException( - repo, - "Failed to update repo " + repo.address + - " - HTTP response " + status); - } - } catch (SSLHandshakeException e) { - throw new UpdateException( - repo, - "A problem occurred while establishing an SSL " + - "connection. If this problem persists, AND you have a " + - "very old device, you could try using http instead of " + - "https for the repo URL.", - e ); + downloader.download(); } catch (IOException e) { if (downloader != null && downloader.getFile() != null) { downloader.getFile().delete(); @@ -154,7 +134,7 @@ abstract public class RepoUpdater { File indexFile = null; try { - HttpDownloader downloader = downloadIndex(); + Downloader downloader = downloadIndex(); hasChanged = downloader.hasChanged(); if (hasChanged) {