Merge branch 'simplify-internal-api-and-tests' into 'master'

Add non-emulator tests and simplify internal API

A lot of the purely internal API is using constructs which are not needed for internal APIs.  The internal API can be viewed and changed by any contributor, so its better to not cover all possible future uses.  Indeed to keep the codebase simple, it should be the opposite: the app's code should reflect what is actually happening now, not what might happen in the future.

This also adds tests that run on the JVM rather than the emulator.

These commits where originally in !248 but I'm submitting them separately since !248 is too big.

See merge request !250
This commit is contained in:
Hans-Christoph Steiner 2016-04-04 12:28:45 +00:00
commit ac2cfe557d
14 changed files with 205 additions and 206 deletions

View File

@ -28,6 +28,8 @@ dependencies {
}
compile 'io.reactivex:rxjava:1.1.0'
compile 'io.reactivex:rxandroid:0.23.0'
testCompile 'junit:junit:4.12'
}
if (!hasProperty('sourceDeps')) {

View File

@ -84,7 +84,6 @@ import org.fdroid.fdroid.data.Apk;
import org.fdroid.fdroid.data.ApkProvider;
import org.fdroid.fdroid.data.App;
import org.fdroid.fdroid.data.AppProvider;
import org.fdroid.fdroid.data.Credentials;
import org.fdroid.fdroid.data.InstalledAppProvider;
import org.fdroid.fdroid.data.Repo;
import org.fdroid.fdroid.data.RepoProvider;
@ -460,12 +459,7 @@ public class AppDetails extends AppCompatActivity implements ProgressListener, A
localBroadcastManager.registerReceiver(downloaderProgressReceiver,
new IntentFilter(Downloader.LOCAL_ACTION_PROGRESS));
downloadHandler.setProgressListener(this);
if (downloadHandler.getTotalBytes() == 0) {
headerFragment.startProgress();
} else {
headerFragment.updateProgress(downloadHandler.getBytesRead(), downloadHandler.getTotalBytes());
}
headerFragment.startProgress();
}
}
}
@ -556,7 +550,7 @@ public class AppDetails extends AppCompatActivity implements ProgressListener, A
@Override
protected void onDestroy() {
if (downloadHandler != null && !inProcessOfChangingConfiguration) {
downloadHandler.cancel(false);
downloadHandler.cancel();
cleanUpFinishedDownload();
}
inProcessOfChangingConfiguration = false;
@ -857,19 +851,8 @@ public class AppDetails extends AppCompatActivity implements ProgressListener, A
return repo.address;
}
@Nullable
private Credentials getRepoCredentials(Apk apk) {
final String[] projection = {RepoProvider.DataColumns.USERNAME, RepoProvider.DataColumns.PASSWORD};
Repo repo = RepoProvider.Helper.findById(this, apk.repo, projection);
if (repo == null || repo.username == null || repo.password == null) {
return null;
}
return repo.getCredentials();
}
private void startDownload(Apk apk, String repoAddress) {
downloadHandler = new ApkDownloader(getBaseContext(), app, apk, repoAddress);
downloadHandler.setCredentials(getRepoCredentials(apk));
localBroadcastManager.registerReceiver(downloaderProgressReceiver,
new IntentFilter(Downloader.LOCAL_ACTION_PROGRESS));
@ -1553,7 +1536,7 @@ public class AppDetails extends AppCompatActivity implements ProgressListener, A
return;
}
activity.downloadHandler.cancel(true);
activity.downloadHandler.cancel();
activity.cleanUpFinishedDownload();
setProgressVisible(false);
updateViews();

View File

@ -21,8 +21,6 @@ import org.xml.sax.XMLReader;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.net.MalformedURLException;
import java.net.URL;
import java.security.CodeSigner;
import java.security.cert.Certificate;
import java.security.cert.X509Certificate;
@ -54,6 +52,8 @@ public class RepoUpdater {
public static final String PROGRESS_COMMITTING = "committing";
public static final String PROGRESS_DATA_REPO_ADDRESS = "repoAddress";
public final String indexUrl;
@NonNull
private final Context context;
@NonNull
@ -75,6 +75,13 @@ public class RepoUpdater {
this.context = context;
this.repo = repo;
this.persister = new RepoPersister(context, repo);
String url = repo.address + "/index.jar";
String versionName = Utils.getVersionName(context);
if (versionName != null) {
url += "?client_version=" + versionName;
}
this.indexUrl = url;
}
public void setProgressListener(@Nullable ProgressListener progressListener) {
@ -85,29 +92,17 @@ public class RepoUpdater {
return hasChanged;
}
private URL getIndexAddress() throws MalformedURLException {
String urlString = repo.address + "/index.jar";
String versionName = Utils.getVersionName(context);
if (versionName != null) {
urlString += "?client_version=" + versionName;
}
return new URL(urlString);
}
private Downloader downloadIndex() throws UpdateException {
Downloader downloader = null;
try {
downloader = DownloaderFactory.create(context,
getIndexAddress(), File.createTempFile("index-", "-downloaded", context.getCacheDir()),
repo.getCredentials()
);
downloader = DownloaderFactory.create(context, indexUrl);
downloader.setCacheTag(repo.lastetag);
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.
Utils.debugLog(TAG, "Repo index for " + getIndexAddress() + " is up to date (by etag)");
Utils.debugLog(TAG, "Repo index for " + indexUrl + " is up to date (by etag)");
}
} catch (IOException e) {

View File

@ -1,36 +0,0 @@
/*
* Copyright (C) 2015 Christian Morgner (christian.morgner@structr.com)
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 3
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
* MA 02110-1301, USA.
*/
package org.fdroid.fdroid.data;
import java.net.HttpURLConnection;
/**
* Credentials to authenticate HTTP requests. Implementations if this interface
* should encapsulate the authentication of an HTTP request in the authenticate
* method.
*/
public interface Credentials {
/**
* Implement this method to provide authentication for the given connection.
* @param connection the HTTP connection to authenticate
*/
void authenticate(final HttpURLConnection connection);
}

View File

@ -5,7 +5,6 @@ import android.database.Cursor;
import android.text.TextUtils;
import org.fdroid.fdroid.Utils;
import org.fdroid.fdroid.net.auth.HttpBasicCredentials;
import java.net.MalformedURLException;
import java.net.URL;
@ -149,21 +148,6 @@ public class Repo extends ValueObject {
return value;
}
/**
* Returns the credentials for this repo, or null of no authentication
* method is configured.
* @return the credentials or null
*/
public Credentials getCredentials() {
// return the only credentials implementation we have right now
if (!TextUtils.isEmpty(username) && !TextUtils.isEmpty(password)) {
return new HttpBasicCredentials(username, password);
}
return null;
}
public void setValues(ContentValues values) {
if (values.containsKey(RepoProvider.DataColumns._ID)) {

View File

@ -36,7 +36,6 @@ import org.fdroid.fdroid.Utils;
import org.fdroid.fdroid.compat.FileCompat;
import org.fdroid.fdroid.data.Apk;
import org.fdroid.fdroid.data.App;
import org.fdroid.fdroid.data.Credentials;
import org.fdroid.fdroid.data.SanitizedFile;
import java.io.File;
@ -69,7 +68,6 @@ public class ApkDownloader implements AsyncDownloader.Listener {
private ProgressListener listener;
private AsyncDownloader dlWrapper;
private Credentials credentials;
private boolean isComplete;
private final long id = ++downloadIdCounter;
@ -188,7 +186,7 @@ public class ApkDownloader implements AsyncDownloader.Listener {
Utils.debugLog(TAG, "Downloading apk from " + remoteAddress + " to " + localFile);
try {
dlWrapper = DownloaderFactory.createAsync(context, remoteAddress, localFile, credentials, this);
dlWrapper = DownloaderFactory.createAsync(context, remoteAddress, localFile, this);
dlWrapper.download();
return true;
} catch (IOException e) {
@ -253,28 +251,14 @@ public class ApkDownloader implements AsyncDownloader.Listener {
/**
* Attempts to cancel the download (if in progress) and also removes the progress
* listener
*
* @param userRequested - true if the user requested the cancel (via button click), otherwise false.
*/
public void cancel(boolean userRequested) {
public void cancel() {
if (dlWrapper != null) {
dlWrapper.attemptCancel(userRequested);
dlWrapper.attemptCancel();
}
}
public Apk getApk() {
return curApk;
}
public int getBytesRead() {
return dlWrapper != null ? dlWrapper.getBytesRead() : 0;
}
public int getTotalBytes() {
return dlWrapper != null ? dlWrapper.getTotalBytes() : 0;
}
public void setCredentials(final Credentials credentials) {
this.credentials = credentials;
}
}

View File

@ -30,22 +30,14 @@ class AsyncDownloadWrapper extends Handler implements AsyncDownloader {
this.listener = listener;
}
public int getBytesRead() {
return downloader.getBytesRead();
}
public int getTotalBytes() {
return downloader.getTotalBytes();
}
public void download() {
downloadThread = new DownloadThread();
downloadThread.start();
}
public void attemptCancel(boolean userRequested) {
if (downloadThread != null) {
downloadThread.interrupt();
public void attemptCancel() {
if (downloader != null) {
downloader.cancelDownload();
}
}

View File

@ -10,12 +10,8 @@ public interface AsyncDownloader {
void onDownloadComplete();
}
int getBytesRead();
int getTotalBytes();
void download();
void attemptCancel(boolean userRequested);
void attemptCancel();
}

View File

@ -23,14 +23,14 @@ public abstract class Downloader {
public static final String EXTRA_BYTES_READ = "extraBytesRead";
public static final String EXTRA_TOTAL_BYTES = "extraTotalBytes";
private volatile boolean cancelled = false;
private final OutputStream outputStream;
private final File outputFile;
protected final URL sourceUrl;
protected String cacheTag;
private int bytesRead;
private int totalBytes;
interface DownloaderProgressListener {
void sendProgress(URL sourceUrl, int bytesRead, int totalBytes);
@ -124,12 +124,19 @@ public abstract class Downloader {
* @throws InterruptedException
*/
private void throwExceptionIfInterrupted() throws InterruptedException {
if (Thread.interrupted()) {
if (cancelled) {
Utils.debugLog(TAG, "Received interrupt, cancelling download");
throw new InterruptedException();
}
}
/**
* Cancel a running download, triggering an {@link InterruptedException}
*/
public void cancelDownload() {
cancelled = true;
}
/**
* This copies the downloaded data from the InputStream to the OutputStream,
* keeping track of the number of bytes that have flowed through for the
@ -138,7 +145,7 @@ public abstract class Downloader {
private void copyInputToOutputStream(InputStream input, int bufferSize) throws IOException, InterruptedException {
int bytesRead = 0;
this.totalBytes = totalDownloadSize();
int totalBytes = totalDownloadSize();
byte[] buffer = new byte[bufferSize];
// Getting the total download size could potentially take time, depending on how
@ -173,20 +180,11 @@ public abstract class Downloader {
}
private void sendProgress(int bytesRead, int totalBytes) {
this.bytesRead = bytesRead;
if (downloaderProgressListener != null) {
downloaderProgressListener.sendProgress(sourceUrl, bytesRead, totalBytes);
}
}
public int getBytesRead() {
return bytesRead;
}
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,

View File

@ -4,7 +4,9 @@ import android.content.Context;
import android.content.Intent;
import android.support.v4.content.LocalBroadcastManager;
import org.fdroid.fdroid.data.Credentials;
import org.apache.commons.io.FilenameUtils;
import org.fdroid.fdroid.data.Repo;
import org.fdroid.fdroid.data.RepoProvider;
import java.io.File;
import java.io.IOException;
@ -23,10 +25,10 @@ public class DownloaderFactory {
throws IOException {
File destFile = File.createTempFile("dl-", "", context.getCacheDir());
destFile.deleteOnExit(); // this probably does nothing, but maybe...
return create(context, new URL(urlString), destFile, null);
return create(context, new URL(urlString), destFile);
}
public static Downloader create(Context context, URL url, File destFile, Credentials credentials)
public static Downloader create(Context context, URL url, File destFile)
throws IOException {
Downloader downloader = null;
if (localBroadcastManager == null) {
@ -39,7 +41,14 @@ public class DownloaderFactory {
} else if (isLocalFile(url)) {
downloader = new LocalFileDownloader(url, destFile);
} else {
downloader = new HttpDownloader(url, destFile, credentials);
final String[] projection = {RepoProvider.DataColumns.USERNAME, RepoProvider.DataColumns.PASSWORD};
String repoUrlString = FilenameUtils.getBaseName(url.toString());
Repo repo = RepoProvider.Helper.findByAddress(context, repoUrlString, projection);
if (repo == null) {
downloader = new HttpDownloader(url, destFile);
} else {
downloader = new HttpDownloader(url, destFile, repo.username, repo.password);
}
}
downloader.setListener(new Downloader.DownloaderProgressListener() {
@ -63,9 +72,9 @@ public class DownloaderFactory {
return "file".equalsIgnoreCase(url.getProtocol());
}
public static AsyncDownloader createAsync(Context context, String urlString, File destFile, Credentials credentials, AsyncDownloader.Listener listener)
public static AsyncDownloader createAsync(Context context, String urlString, File destFile, AsyncDownloader.Listener listener)
throws IOException {
URL url = new URL(urlString);
return new AsyncDownloadWrapper(create(context, url, destFile, credentials), listener);
return new AsyncDownloadWrapper(create(context, url, destFile), listener);
}
}

View File

@ -6,7 +6,7 @@ import com.nostra13.universalimageloader.core.download.BaseImageDownloader;
import org.fdroid.fdroid.FDroidApp;
import org.fdroid.fdroid.Utils;
import org.fdroid.fdroid.data.Credentials;
import org.spongycastle.util.encoders.Base64;
import java.io.BufferedInputStream;
import java.io.File;
@ -28,20 +28,33 @@ public class HttpDownloader extends Downloader {
protected static final String HEADER_IF_NONE_MATCH = "If-None-Match";
protected static final String HEADER_FIELD_ETAG = "ETag";
public final String username;
public final String password;
protected HttpURLConnection connection;
private Credentials credentials;
private int statusCode = -1;
HttpDownloader(URL url, File destFile)
throws FileNotFoundException, MalformedURLException {
this(url, destFile, null);
this(url, destFile, null, null);
}
HttpDownloader(URL url, File destFile, final Credentials credentials)
/**
* Create a downloader that can authenticate via HTTP Basic Auth using the supplied
* {@code username} and {@code password}.
*
* @param url The file to download
* @param destFile Where the download is saved
* @param username Username for HTTP Basic Auth, use {@code null} to ignore
* @param password Password for HTTP Basic Auth, use {@code null} to ignore
* @throws FileNotFoundException
* @throws MalformedURLException
*/
HttpDownloader(URL url, File destFile, String username, String password)
throws FileNotFoundException, MalformedURLException {
super(url, destFile);
this.credentials = credentials;
this.username = username;
this.password = password;
}
/**
@ -106,8 +119,10 @@ public class HttpDownloader extends Downloader {
((HttpsURLConnection) connection).setSSLSocketFactory(HttpsURLConnection.getDefaultSSLSocketFactory());
}
if (credentials != null) {
credentials.authenticate(connection);
if (username != null && password != null) {
// add authorization header from username / password if set
String authString = username + ":" + password;
connection.setRequestProperty("Authorization", "Basic " + Base64.toBase64String(authString.getBytes()));
}
}

View File

@ -1,51 +0,0 @@
/*
* Copyright (C) 2015 Christian Morgner (christian.morgner@structr.com)
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 3
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
* MA 02110-1301, USA.
*/
package org.fdroid.fdroid.net.auth;
import android.text.TextUtils;
import org.apache.commons.net.util.Base64;
import org.fdroid.fdroid.data.Credentials;
import java.net.HttpURLConnection;
/**
* Credentials implementation for HTTP Basic Authentication.
*/
public class HttpBasicCredentials implements Credentials {
private final String username;
private final String password;
public HttpBasicCredentials(final String username, final String password) {
this.username = username;
this.password = password;
}
@Override
public void authenticate(final HttpURLConnection connection) {
if (!TextUtils.isEmpty(username) && !TextUtils.isEmpty(password)) {
// add authorization header from username / password if set
connection.setRequestProperty("Authorization", "Basic " + Base64.encodeBase64String((username + ":" + password).getBytes()));
}
}
}

View File

@ -1,14 +1,14 @@
package org.fdroid.fdroid;
package org.fdroid.fdroid.data;
import android.test.AndroidTestCase;
import org.fdroid.fdroid.data.SanitizedFile;
import org.junit.Test;
import java.io.File;
@SuppressWarnings("PMD") // TODO port this to JUnit 4 semantics
public class SanitizedFileTest extends AndroidTestCase {
import static org.junit.Assert.assertEquals;
public class SanitizedFileTest {
@Test
public void testSanitizedFile() {
File directory = new File("/tmp/blah");

View File

@ -0,0 +1,128 @@
package org.fdroid.fdroid.net;
import org.junit.Test;
import java.io.File;
import java.io.IOException;
import java.net.URL;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
public class HttpDownloaderTest {
String[] urls = {
"https://www.google.com",
"https://en.wikipedia.org/wiki/Index.html",
"https://mirrors.kernel.org/debian/dists/stable/Release",
"https://f-droid.org/archive/de.we.acaldav_5.apk",
// sites that use SNI for HTTPS
"https://guardianproject.info/fdroid/repo/index.jar",
"https://firstlook.org",
};
private boolean receivedProgress;
@Test
public void downloadUninterruptedTest() throws IOException, InterruptedException {
for (String urlString : urls) {
URL url = new URL(urlString);
File destFile = File.createTempFile("dl-", "");
destFile.deleteOnExit(); // this probably does nothing, but maybe...
HttpDownloader httpDownloader = new HttpDownloader(url, destFile);
httpDownloader.download();
assertTrue(destFile.exists());
assertTrue(destFile.canRead());
}
}
@Test
public void downloadUninterruptedTestWithProgress() throws IOException, InterruptedException {
for (String urlString : urls) {
receivedProgress = false;
URL url = new URL(urlString);
File destFile = File.createTempFile("dl-", "");
destFile.deleteOnExit(); // this probably does nothing, but maybe...
HttpDownloader httpDownloader = new HttpDownloader(url, destFile);
httpDownloader.setListener(new Downloader.DownloaderProgressListener() {
@Override
public void sendProgress(URL sourceUrl, int bytesRead, int totalBytes) {
receivedProgress = true;
}
});
httpDownloader.download();
assertTrue(destFile.exists());
assertTrue(destFile.canRead());
assertTrue(receivedProgress);
}
}
@Test
public void downloadHttpBasicAuth() throws IOException, InterruptedException {
URL url = new URL("https://httpbin.org/basic-auth/myusername/supersecretpassword");
File destFile = File.createTempFile("dl-", "");
destFile.deleteOnExit(); // this probably does nothing, but maybe...
HttpDownloader httpDownloader = new HttpDownloader(url, destFile, "myusername", "supersecretpassword");
httpDownloader.download();
assertTrue(destFile.exists());
assertTrue(destFile.canRead());
}
@Test(expected = IOException.class)
public void downloadHttpBasicAuthWrongPassword() throws IOException, InterruptedException {
URL url = new URL("https://httpbin.org/basic-auth/myusername/supersecretpassword");
File destFile = File.createTempFile("dl-", "");
destFile.deleteOnExit(); // this probably does nothing, but maybe...
HttpDownloader httpDownloader = new HttpDownloader(url, destFile, "myusername", "wrongpassword");
httpDownloader.download();
assertFalse(destFile.exists());
}
@Test(expected = IOException.class)
public void downloadHttpBasicAuthWrongUsername() throws IOException, InterruptedException {
URL url = new URL("https://httpbin.org/basic-auth/myusername/supersecretpassword");
File destFile = File.createTempFile("dl-", "");
destFile.deleteOnExit(); // this probably does nothing, but maybe...
HttpDownloader httpDownloader = new HttpDownloader(url, destFile, "wrongusername", "supersecretpassword");
httpDownloader.download();
assertFalse(destFile.exists());
}
@Test
public void downloadThenCancel() throws IOException, InterruptedException {
final CountDownLatch latch = new CountDownLatch(5);
URL url = new URL("https://f-droid.org/repo/index.jar");
File destFile = File.createTempFile("dl-", "");
destFile.deleteOnExit(); // this probably does nothing, but maybe...
final HttpDownloader httpDownloader = new HttpDownloader(url, destFile);
httpDownloader.setListener(new Downloader.DownloaderProgressListener() {
@Override
public void sendProgress(URL sourceUrl, int bytesRead, int totalBytes) {
System.out.println("DownloaderProgressListener.sendProgress " + bytesRead + " / " + totalBytes);
receivedProgress = true;
latch.countDown();
}
});
new Thread() {
@Override
public void run() {
try {
httpDownloader.download();
fail();
} catch (IOException e) {
e.printStackTrace();
fail();
} catch (InterruptedException e) {
// success!
}
}
}.start();
latch.await(100, TimeUnit.SECONDS); // either 5 progress reports or 100 seconds
httpDownloader.cancelDownload();
assertTrue(receivedProgress);
}
}