Merge branch 'improve-downloader-close-handling' into 'master'

Make closing of `Downloader`s more concise.

*NOTE: This is  only a WIP branch in so far as I haven't tested it, as it was done on the train before my holiday. The code is final though, so if people are happy it works, please merge.*

The base `Downloader` class now wraps the `InputStream` returned by
any child classes, in order to notify the child class when that stream
is closed. This prevents each child class having to figure out a way
to be notified of this.

This helps keep the API concise, because close handling is dealt with
without the need to add any public methods to the `Downloader` class
hierarchy.

Also removed some dead code which was unused.

See merge request !139

F-Droid/src/org/fdroid/fdroid/RepoUpdater.java

@@ -108,7 +108,6 @@ public class RepoUpdater {
         } catch (IOException e) {
             if (downloader != null && downloader.getFile() != null) {
                 downloader.getFile().delete();
-                downloader.close();
             }
 
             throw new UpdateException(repo, "Error getting index file from " + repo.address, e);
@@ -133,8 +132,6 @@ public class RepoUpdater {
             // successful download, then we will have a file ready to use:
             processDownloadedFile(downloader.getFile(), downloader.getCacheTag());
         }
-
-        downloader.close();
     }
 
     protected void processDownloadedFile(File downloadedFile, String cacheTag) throws UpdateException {

F-Droid/src/org/fdroid/fdroid/net/BluetoothDownloader.java

@@ -1,6 +1,7 @@
 package org.fdroid.fdroid.net;
 
 import android.content.Context;
+import android.support.annotation.Nullable;
 import android.util.Log;
 
 import org.apache.commons.io.input.BoundedInputStream;
@@ -31,12 +32,11 @@ public class BluetoothDownloader extends Downloader {
     }
 
     @Override
-    public InputStream getInputStream() throws IOException {
+    protected InputStream getDownloadersInputStream() throws IOException {
         Request request = Request.createGET(sourcePath, connection);
         Response response = request.send();
         fileDetails = response.toFileDetails();
 
-
         // TODO: Manage the dependency which includes this class better?
         // Right now, I only needed the one class from apache commons.
         // There are countless classes online which provide this functionality,
@@ -53,10 +53,8 @@ public class BluetoothDownloader extends Downloader {
 
     /**
      * May return null if an error occurred while getting file details.
-     * TODO: Should we throw an exception? Everywhere else in this blue package throws IO exceptions weely-neely.
-     * Will probably require some thought as to how the API looks, with regards to all of the public methods
-     * and their signatures.
      */
+    @Nullable
     public FileDetails getFileDetails() {
         if (fileDetails == null) {
             Utils.debugLog(TAG, "Going to Bluetooth \"server\" to get file details.");
@@ -71,12 +69,14 @@ public class BluetoothDownloader extends Downloader {
 
     @Override
     public boolean hasChanged() {
-        return getFileDetails().getCacheTag() == null || getFileDetails().getCacheTag().equals(getCacheTag());
+        FileDetails details = getFileDetails();
+        return details == null || details.getCacheTag() == null || details.getCacheTag().equals(getCacheTag());
     }
 
     @Override
     public int totalDownloadSize() {
-        return getFileDetails().getFileSize();
+        FileDetails details = getFileDetails();
+        return details != null ? details.getFileSize() : -1;
     }
 
     @Override
@@ -94,7 +94,7 @@ public class BluetoothDownloader extends Downloader {
     }
 
     @Override
-    public void close() {
+    protected void close() {
         if (connection != null)
             connection.closeQuietly();
     }

F-Droid/src/org/fdroid/fdroid/net/Downloader.java

@@ -2,6 +2,7 @@ package org.fdroid.fdroid.net;
 
 import android.content.Context;
 import android.content.Intent;
+import android.support.annotation.NonNull;
 import android.support.v4.content.LocalBroadcastManager;
 
 import org.fdroid.fdroid.Utils;
@@ -35,9 +36,9 @@ public abstract class Downloader {
     protected int bytesRead;
     protected int totalBytes;
 
-    public abstract InputStream getInputStream() throws IOException;
+    protected abstract InputStream getDownloadersInputStream() throws IOException;
 
-    public abstract void close();
+    protected abstract void close() throws IOException;
 
     Downloader(Context context, URL url, File destFile)
             throws FileNotFoundException, MalformedURLException {
@@ -47,6 +48,10 @@ public abstract class Downloader {
         localBroadcastManager = LocalBroadcastManager.getInstance(context);
     }
 
+    public final InputStream getInputStream() throws IOException {
+        return new WrappedInputStream(getDownloadersInputStream());
+    }
+
     /**
      * 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
@@ -197,4 +202,66 @@ public abstract class Downloader {
     public int getTotalBytes() {
         return totalBytes;
     }
+
+    /**
+     * Overrides every method in {@link InputStream} and delegates to the wrapped stream.
+     * The only difference is that when we call the {@link WrappedInputStream#close()} method,
+     * after delegating to the wrapped stream we invoke the {@link Downloader#close()} method
+     * on the {@link Downloader} which created this.
+     */
+    private class WrappedInputStream extends InputStream {
+
+        private InputStream toWrap;
+
+        public WrappedInputStream(InputStream toWrap) {
+            super();
+            this.toWrap = toWrap;
+        }
+
+        @Override
+        public void close() throws IOException {
+            toWrap.close();
+            Downloader.this.close();
+        }
+
+        @Override
+        public int available() throws IOException {
+            return toWrap.available();
+        }
+
+        @Override
+        public void mark(int readlimit) {
+            toWrap.mark(readlimit);
+        }
+
+        @Override
+        public boolean markSupported() {
+            return toWrap.markSupported();
+        }
+
+        @Override
+        public int read(@NonNull byte[] buffer) throws IOException {
+            return toWrap.read(buffer);
+        }
+
+        @Override
+        public int read(@NonNull byte[] buffer, int byteOffset, int byteCount) throws IOException {
+            return toWrap.read(buffer, byteOffset, byteCount);
+        }
+
+        @Override
+        public synchronized void reset() throws IOException {
+            toWrap.reset();
+        }
+
+        @Override
+        public long skip(long byteCount) throws IOException {
+            return toWrap.skip(byteCount);
+        }
+
+        @Override
+        public int read() throws IOException {
+            return toWrap.read();
+        }
+    }
 }

F-Droid/src/org/fdroid/fdroid/net/DownloaderFactory.java

@@ -108,6 +108,10 @@ public class DownloaderFactory {
             // We support onion addresses through our own downloader.
             return false;
         }
+        if (isBluetoothAddress(url)) {
+            // Completely differnet protocol not understood by the download manager.
+            return false;
+        }
         return hasDownloadManager(context);
     }
 

F-Droid/src/org/fdroid/fdroid/net/HttpDownloader.java

@@ -8,12 +8,10 @@ import org.fdroid.fdroid.Preferences;
 import org.fdroid.fdroid.Utils;
 
 import java.io.BufferedInputStream;
-import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.InputStreamReader;
 import java.net.HttpURLConnection;
 import java.net.InetSocketAddress;
 import java.net.MalformedURLException;
@@ -30,7 +28,6 @@ public class HttpDownloader extends Downloader {
     protected static final String HEADER_FIELD_ETAG = "ETag";
 
     protected HttpURLConnection connection;
-    private InputStream stream;
     private int statusCode = -1;
 
     HttpDownloader(Context context, URL url, File destFile)
@@ -38,14 +35,6 @@ public class HttpDownloader extends Downloader {
         super(context, url, destFile);
     }
 
-    /**
-     * Calling this makes this downloader not download a file. Instead, it will
-     * only stream the file through the {@link HttpDownloader#getInputStream()}
-     */
-    public HttpDownloader streamDontDownload() {
-        return this;
-    }
-
     /**
      * Note: Doesn't follow redirects (as far as I'm aware).
      * {@link BaseImageDownloader#getStreamFromNetwork(String, Object)} has an implementation worth
@@ -55,14 +44,9 @@ public class HttpDownloader extends Downloader {
      * @throws IOException
      */
 
-    public InputStream getInputStream() throws IOException {
+    protected InputStream getDownloadersInputStream() throws IOException {
         setupConnection();
-        stream = new BufferedInputStream(connection.getInputStream());
+        return new BufferedInputStream(connection.getInputStream());
-        return stream;
-    }
-
-    public BufferedReader getBufferedReader() throws IOException {
-        return new BufferedReader(new InputStreamReader(getInputStream()));
     }
 
     // Get a remote file. Returns the HTTP response code.
@@ -155,18 +139,8 @@ public class HttpDownloader extends Downloader {
         return this.statusCode != 304;
     }
 
-    public int getStatusCode() {
+    @Override
-        return statusCode;
-    }
-
     public void close() {
-        try {
-            if (stream != null)
-                stream.close();
-        } catch (IOException e) {
-            // ignore
-        }
-
         connection.disconnect();
     }
 }

F-Droid/src/org/fdroid/fdroid/net/IconDownloader.java

@@ -4,11 +4,8 @@ import android.content.Context;
 
 import com.nostra13.universalimageloader.core.download.BaseImageDownloader;
 
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
-import java.util.Locale;
 
 public class IconDownloader extends BaseImageDownloader {
 
@@ -22,38 +19,7 @@ public class IconDownloader extends BaseImageDownloader {
 
     @Override
     public InputStream getStream(String imageUri, Object extra) throws IOException {
-
+        return DownloaderFactory.create(context, imageUri).getInputStream();
-        Scheme scheme = Scheme.ofUri(imageUri);
-
-        switch (scheme) {
-            case HTTP:
-            case HTTPS:
-                Downloader downloader = DownloaderFactory.create(context, imageUri);
-                return downloader.getInputStream();
-        }
-
-        //bluetooth isn't a scheme in the Scheme. library, so we can add a check here
-        if (imageUri.toLowerCase(Locale.ENGLISH).startsWith("bluetooth")) {
-            Downloader downloader = DownloaderFactory.create(context, imageUri);
-
-            ByteArrayOutputStream baos = new ByteArrayOutputStream();
-            InputStream is = downloader.getInputStream();
-
-            int b;
-            while ((b = is.read()) != -1)
-                baos.write(b);
-
-            ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray());
-
-            downloader.close();
-
-            return bais;
-
-        }
-
-
-        return super.getStream(imageUri, extra);
-
     }
 
 

F-Droid/src/org/fdroid/fdroid/views/swap/SwapAppsView.java

@@ -1,6 +1,7 @@
 package org.fdroid.fdroid.views.swap;
 
 import android.annotation.TargetApi;
+import android.app.Activity;
 import android.content.BroadcastReceiver;
 import android.content.Context;
 import android.content.Intent;
@@ -330,9 +331,12 @@ public class SwapAppsView extends ListView implements
             private final ContentObserver appObserver = new ContentObserver(new Handler()) {
                 @Override
                 public void onChange(boolean selfChange) {
-                    app = AppProvider.Helper.findById(getActivity().getContentResolver(), app.id);
+                    Activity activity = getActivity();
-                    apkToInstall = null; // Force lazy loading to fetch correct apk next time.
+                    if (activity != null) {
-                    resetView();
+                        app = AppProvider.Helper.findById(getActivity().getContentResolver(), app.id);
+                        apkToInstall = null; // Force lazy loading to fetch correct apk next time.
+                        resetView();
+                    }
                 }
             };