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

@@ -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;
             }

src/org/fdroid/fdroid/net/Downloader.java

@@ -1,17 +1,29 @@
 package org.fdroid.fdroid.net;
 
-import java.io.*;
+import android.content.Context;
-import java.net.*;
+import android.util.Log;
-import android.content.*;
+
-import org.fdroid.fdroid.*;
+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();
+    public void download() throws IOException {
-
+        Log.i(TAG, "download");
-    protected void download() throws IOException {
         setupProgressListener();
         InputStream input = null;
         try {

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 {
+    @Override
+    public void download() throws IOException {
+        try {
             connection = (HttpURLConnection)sourceUrl.openConnection();
             setupCacheCheck();
             statusCode = connection.getResponseCode();
-        if (statusCode == 200) {
+            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;
     }
 
-    /**
+    private void setupCacheCheck() {
-     * 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() {
         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;
     }

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 {
+    protected Downloader downloadIndex() throws UpdateException {
-        HttpDownloader downloader = null;
+        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();
+            downloader.download();
-
-            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 );
         } 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) {