Make closing of Downloaders more concise.

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.

Also removed some dead code which was unused.
This commit is contained in:
Peter Serwylo 2015-09-14 07:43:19 +10:00
parent c6a8bd2139
commit 012cdce37b
5 changed files with 50 additions and 77 deletions

View File

@ -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 {

View File

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

View File

@ -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,35 @@ 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(); }
}
}

View File

@ -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 stream;
}
public BufferedReader getBufferedReader() throws IOException {
return new BufferedReader(new InputStreamReader(getInputStream()));
return new BufferedInputStream(connection.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() {
return statusCode;
}
@Override
public void close() {
try {
if (stream != null)
stream.close();
} catch (IOException e) {
// ignore
}
connection.disconnect();
}
}

View File

@ -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 {
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);
return DownloaderFactory.create(context, imageUri).getInputStream();
}