Add glide support to download library

This commit is contained in:
Torsten Grote 2022-01-19 15:07:46 -03:00
parent 5b5cd42384
commit a43d5d8ef1
No known key found for this signature in database
GPG Key ID: 3E5F77D92CF891FF
19 changed files with 364 additions and 33 deletions

View File

@ -177,8 +177,8 @@ dependencies {
implementation 'com.fasterxml.jackson.core:jackson-annotations:2.11.1'
implementation 'com.fasterxml.jackson.core:jackson-databind:2.11.1'
implementation 'com.github.bumptech.glide:glide:4.12.0'
annotationProcessor 'com.github.bumptech.glide:compiler:4.12.0'
implementation "com.github.bumptech.glide:glide:$glide_version"
annotationProcessor "com.github.bumptech.glide:compiler:$glide_version"
implementation 'org.bouncycastle:bcprov-jdk15on:1.65'
fullImplementation 'org.bouncycastle:bcpkix-jdk15on:1.65'

View File

@ -125,7 +125,8 @@ public class HttpDownloaderTest {
String path = "myusername/supersecretpassword";
List<Mirror> mirrors = Mirror.fromStrings(Collections.singletonList("https://httpbin.org/basic-auth/"));
File destFile = File.createTempFile("dl-", "");
final DownloadRequest request = new DownloadRequest(path, mirrors, null,"myusername", "wrongpassword");
final DownloadRequest request =
new DownloadRequest(path, mirrors, null, "myusername", "wrongpassword");
HttpDownloader httpDownloader = new HttpDownloader(httpManager, request, destFile);
httpDownloader.download();
assertFalse(destFile.exists());
@ -137,7 +138,8 @@ public class HttpDownloaderTest {
String path = "myusername/supersecretpassword";
List<Mirror> mirrors = Mirror.fromStrings(Collections.singletonList("https://httpbin.org/basic-auth/"));
File destFile = File.createTempFile("dl-", "");
final DownloadRequest request = new DownloadRequest(path, mirrors, null, "wrongusername", "supersecretpassword");
final DownloadRequest request =
new DownloadRequest(path, mirrors, null, "wrongusername", "supersecretpassword");
HttpDownloader httpDownloader = new HttpDownloader(httpManager, request, destFile);
httpDownloader.download();
assertFalse(destFile.exists());

View File

@ -313,8 +313,7 @@ public class SwapSuccessView extends SwapView implements LoaderManager.LoaderCal
nameView.setText(app.name);
}
Glide.with(iconView.getContext())
.load(app.getIconUrl(iconView.getContext()))
app.loadWithGlide(iconView.getContext())
.apply(Utils.getAlwaysShowIconRequestOptions())
.into(iconView);

View File

@ -491,7 +491,7 @@ public final class Utils {
.fallback(R.drawable.ic_repo_app_default);
}
iconRequestOptions.onlyRetrieveFromCache(!Preferences.get().isBackgroundDownloadAllowed());
Glide.with(context).load(app.getIconUrl(iv.getContext())).apply(iconRequestOptions).into(iv);
app.loadWithGlide(context).apply(iconRequestOptions).into(iv);
}
/**

View File

@ -10,6 +10,7 @@ import android.content.res.AssetManager;
import android.content.res.Resources;
import android.content.res.XmlResourceParser;
import android.database.Cursor;
import android.graphics.drawable.Drawable;
import android.net.Uri;
import android.os.Build;
import android.os.Environment;
@ -19,11 +20,15 @@ import android.os.Parcelable;
import android.text.TextUtils;
import android.util.Log;
import com.bumptech.glide.Glide;
import com.bumptech.glide.RequestBuilder;
import com.fasterxml.jackson.annotation.JacksonInject;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import org.apache.commons.io.filefilter.RegexFileFilter;
import org.fdroid.download.DownloadRequest;
import org.fdroid.fdroid.FDroidApp;
import org.fdroid.fdroid.Preferences;
import org.fdroid.fdroid.R;
import org.fdroid.fdroid.Utils;
@ -733,8 +738,18 @@ public class App extends ValueObject implements Comparable<App>, Parcelable {
.build();
}
public String getIconUrl(Context context) {
public RequestBuilder<Drawable> loadWithGlide(Context context) {
Repo repo = RepoProvider.Helper.findById(context, repoId);
if (repo.address.startsWith("content://")) {
return Glide.with(context).load(getIconUrl(context, repo));
} else {
return Glide.with(context).load(getDownloadRequest(context, repo));
}
}
@Nullable
@Deprecated // not taking mirrors into account
public String getIconUrl(Context context, Repo repo) {
if (TextUtils.isEmpty(iconUrl)) {
if (TextUtils.isEmpty(iconFromApk)) {
return null;
@ -755,6 +770,44 @@ public class App extends ValueObject implements Comparable<App>, Parcelable {
return repo.getFileUrl(packageName, iconUrl);
}
@Nullable
@Deprecated // not taking mirrors into account
public String getIconUrl(Context context) {
Repo repo = RepoProvider.Helper.findById(context, repoId);
return getIconUrl(context, repo);
}
@Nullable
public DownloadRequest getDownloadRequest(Context context, Repo repo) {
String path;
if (TextUtils.isEmpty(iconUrl)) {
if (TextUtils.isEmpty(iconFromApk)) {
return null;
}
if (iconFromApk.endsWith(".xml")) {
// We cannot use xml resources as icons. F-Droid server should not include them
// https://gitlab.com/fdroid/fdroidserver/issues/344
return null;
}
String iconsDir;
if (repo.version >= Repo.VERSION_DENSITY_SPECIFIC_ICONS) {
iconsDir = Utils.getIconsDir(context, 1.0);
} else {
iconsDir = Utils.FALLBACK_ICONS_DIR;
}
path = repo.getPath(iconsDir, iconFromApk);
} else {
path = repo.getPath(packageName, iconUrl);
}
return repo.getDownloadRequest(path);
}
@Nullable
public DownloadRequest getDownloadRequest(Context context) {
Repo repo = RepoProvider.Helper.findById(context, repoId);
return getDownloadRequest(context, repo);
}
public String getFeatureGraphicUrl(Context context) {
if (TextUtils.isEmpty(featureGraphic)) {
return null;

View File

@ -30,18 +30,24 @@ import android.text.TextUtils;
import com.fasterxml.jackson.annotation.JsonIgnore;
import org.fdroid.download.DownloadRequest;
import org.fdroid.download.Mirror;
import org.fdroid.fdroid.Utils;
import org.fdroid.fdroid.data.Schema.RepoTable.Cols;
import org.fdroid.fdroid.net.TreeUriDownloader;
import java.net.MalformedURLException;
import java.net.Proxy;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import info.guardianproject.netcipher.NetCipher;
/**
* Represents a the descriptive info and metadata about a given repo, as provided
@ -268,6 +274,27 @@ public class Repo extends ValueObject {
return tempName;
}
/**
* Gets the path relative to the repo root.
* Can be used to create URLs for use with mirrors.
* Attention: This does NOT encode for use in URLs.
*/
public String getPath(String... pathElements) {
/* Each String in pathElements might contain a /, should keep these as path elements */
ArrayList<String> elements = new ArrayList<>();
for (String element : pathElements) {
Collections.addAll(elements, element.split("/"));
}
// build up path WITHOUT encoding the segments, this will happen later when turned into URL
StringBuilder sb = new StringBuilder();
for (String element : elements) {
sb.append(element).append("/");
}
sb.deleteCharAt(sb.length() - 1); // remove trailing slash
return sb.toString();
}
@Deprecated // not taking mirrors into account
public String getFileUrl(String... pathElements) {
/* Each String in pathElements might contain a /, should keep these as path elements */
List<String> elements = new ArrayList();
@ -302,6 +329,12 @@ public class Repo extends ValueObject {
}
}
public DownloadRequest getDownloadRequest(String path) {
List<Mirror> mirrors = Mirror.fromStrings(getMirrorList());
Proxy proxy = NetCipher.getProxy();
return new DownloadRequest(path, mirrors, proxy, username, password);
}
private static int toInt(Integer value) {
if (value == null) {
return 0;

View File

@ -4,19 +4,29 @@ import android.content.Context;
import android.graphics.Bitmap;
import android.graphics.drawable.Drawable;
import com.bumptech.glide.Glide;
import com.bumptech.glide.GlideBuilder;
import com.bumptech.glide.Registry;
import com.bumptech.glide.annotation.GlideModule;
import com.bumptech.glide.load.DecodeFormat;
import com.bumptech.glide.load.model.GlideUrl;
import com.bumptech.glide.load.resource.bitmap.BitmapTransitionOptions;
import com.bumptech.glide.load.resource.drawable.DrawableTransitionOptions;
import com.bumptech.glide.module.AppGlideModule;
import com.bumptech.glide.request.RequestOptions;
import org.fdroid.download.DownloadRequest;
import org.fdroid.download.glide.DownloadRequestLoader;
import org.fdroid.download.glide.HttpGlideUrlLoader;
import org.fdroid.fdroid.FDroidApp;
import org.fdroid.fdroid.Preferences;
import androidx.annotation.NonNull;
import java.io.InputStream;
import info.guardianproject.netcipher.NetCipher;
/**
* The one time initialization of Glide.
*/
@ -32,4 +42,14 @@ public class FDroidGlideModule extends AppGlideModule {
.onlyRetrieveFromCache(!Preferences.get().isBackgroundDownloadAllowed())
.timeout(FDroidApp.getTimeout()));
}
@Override
public void registerComponents(@NonNull Context context, @NonNull Glide glide, Registry registry) {
HttpGlideUrlLoader.Factory urlLoaderFactory =
new HttpGlideUrlLoader.Factory(DownloaderFactory.HTTP_MANAGER, NetCipher::getProxy);
registry.replace(GlideUrl.class, InputStream.class, urlLoaderFactory);
DownloadRequestLoader.Factory requestLoaderFactory =
new DownloadRequestLoader.Factory(DownloaderFactory.HTTP_MANAGER);
registry.append(DownloadRequest.class, InputStream.class, requestLoaderFactory);
}
}

View File

@ -79,8 +79,7 @@ public class InstallConfirmActivity extends AppCompatActivity implements OnCance
TabHost tabHost = (TabHost) findViewById(android.R.id.tabhost);
appName.setText(app.name);
Glide.with(this)
.load(app.getIconUrl(this))
app.loadWithGlide(this)
.apply(Utils.getAlwaysShowIconRequestOptions())
.into(appIcon);

View File

@ -1,6 +1,7 @@
buildscript {
ext {
kotlin_version = "1.6.10"
glide_version = "4.12.0"
}
repositories {
mavenCentral()

View File

@ -56,6 +56,10 @@ kotlin {
androidMain {
dependencies {
implementation "io.ktor:ktor-client-okhttp:$ktor_version"
implementation("com.github.bumptech.glide:glide:$glide_version") {
transitive = false // we don't need all that it pulls in, just the basics
}
implementation "com.github.bumptech.glide:annotations:$glide_version"
implementation 'ch.qos.logback:logback-classic:1.2.5'
}
}

View File

@ -110,6 +110,7 @@ abstract class Downloader constructor(
}
@Suppress("BlockingMethodInNonBlockingContext")
@Throws(InterruptedException::class, IOException::class, NoResumeException::class)
protected suspend fun downloadFromBytesReceiver(isResume: Boolean) {
try {
FileOutputStream(outputFile, isResume).use { outputStream ->

View File

@ -65,7 +65,7 @@ class HttpDownloader constructor(
throw NotImplementedError("Use getInputStreamSuspend instead.")
}
@Throws(IOException::class)
@Throws(IOException::class, NoResumeException::class)
override suspend fun getBytes(resumable: Boolean, receiver: (ByteArray) -> Unit) {
val skipBytes = if (resumable) outputFile.length() else null
return try {
@ -161,7 +161,15 @@ class HttpDownloader constructor(
resumable = true
}
log.debug { "downloading ${request.path} (is resumable: $resumable)" }
runBlocking { downloadFromBytesReceiver(resumable) }
runBlocking {
try {
downloadFromBytesReceiver(resumable)
} catch (e: NoResumeException) {
require(resumable) { "Got $e even though download was not resumable" }
if (!outputFile.delete()) log.warn { "Warning: " + outputFile.absolutePath + " not deleted" }
downloadFromBytesReceiver(false)
}
}
}
@TargetApi(24)

View File

@ -0,0 +1,45 @@
package org.fdroid.download.glide
import com.bumptech.glide.load.Options
import com.bumptech.glide.load.model.ModelLoader
import com.bumptech.glide.load.model.ModelLoader.LoadData
import com.bumptech.glide.load.model.ModelLoaderFactory
import com.bumptech.glide.load.model.MultiModelLoaderFactory
import com.bumptech.glide.signature.ObjectKey
import org.fdroid.download.DownloadRequest
import org.fdroid.download.HttpManager
import java.io.InputStream
class DownloadRequestLoader(
private val httpManager: HttpManager,
) : ModelLoader<DownloadRequest, InputStream> {
override fun handles(downloadRequest: DownloadRequest): Boolean {
return true
}
override fun buildLoadData(
downloadRequest: DownloadRequest,
width: Int,
height: Int,
options: Options,
): LoadData<InputStream> {
return LoadData(downloadRequest.getKey(), HttpFetcher(httpManager, downloadRequest))
}
class Factory(
private val httpManager: HttpManager,
) : ModelLoaderFactory<DownloadRequest, InputStream> {
override fun build(multiFactory: MultiModelLoaderFactory): ModelLoader<DownloadRequest, InputStream> {
return DownloadRequestLoader(httpManager)
}
override fun teardown() {}
}
}
fun DownloadRequest.getKey(): ObjectKey {
// TODO should we choose a unique key or is it ok for this to work cross-repo based on file path only?
return ObjectKey(path)
}

View File

@ -0,0 +1,68 @@
package org.fdroid.download.glide
import android.net.Uri
import com.bumptech.glide.Priority
import com.bumptech.glide.load.DataSource
import com.bumptech.glide.load.data.DataFetcher
import com.bumptech.glide.load.model.GlideUrl
import io.ktor.client.engine.ProxyConfig
import io.ktor.utils.io.jvm.javaio.toInputStream
import kotlinx.coroutines.DelicateCoroutinesApi
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.launch
import org.fdroid.download.DownloadRequest
import org.fdroid.download.HttpManager
import org.fdroid.download.Mirror
import java.io.InputStream
class HttpFetcher(
private val httpManager: HttpManager,
private val downloadRequest: DownloadRequest,
) : DataFetcher<InputStream> {
@Deprecated("Use DownloadRequests with other constructor instead")
constructor(
httpManager: HttpManager,
glideUrl: GlideUrl, proxy: ProxyConfig?,
) : this(httpManager, getDownloadRequest(glideUrl, proxy))
companion object {
private fun getDownloadRequest(glideUrl: GlideUrl, proxy: ProxyConfig?): DownloadRequest {
val (mirror, path) = glideUrl.toStringUrl().split("/repo/")
return DownloadRequest(Uri.decode(path), listOf(Mirror("$mirror/repo")), proxy)
}
}
var job: Job? = null
@OptIn(DelicateCoroutinesApi::class)
override fun loadData(priority: Priority, callback: DataFetcher.DataCallback<in InputStream>) {
job = GlobalScope.launch(Dispatchers.IO) {
try {
// glide should take care of closing this stream and the underlying channel
val inputStream = httpManager.getChannel(downloadRequest).toInputStream()
callback.onDataReady(inputStream)
} catch (e: Exception) {
callback.onLoadFailed(e)
}
}
}
override fun cleanup() {
job = null
}
override fun cancel() {
job?.cancel()
}
override fun getDataClass(): Class<InputStream> {
return InputStream::class.java
}
override fun getDataSource(): DataSource {
return DataSource.REMOTE
}
}

View File

@ -0,0 +1,44 @@
package org.fdroid.download.glide
import com.bumptech.glide.load.Options
import com.bumptech.glide.load.model.GlideUrl
import com.bumptech.glide.load.model.ModelLoader
import com.bumptech.glide.load.model.ModelLoader.LoadData
import com.bumptech.glide.load.model.ModelLoaderFactory
import com.bumptech.glide.load.model.MultiModelLoaderFactory
import io.ktor.client.engine.ProxyConfig
import mu.KotlinLogging
import org.fdroid.download.HttpManager
import java.io.InputStream
@Deprecated("Use DownloadRequestLoader instead")
class HttpGlideUrlLoader(
private val httpManager: HttpManager,
private val proxyGetter: () -> ProxyConfig?,
) : ModelLoader<GlideUrl, InputStream> {
companion object {
private val log = KotlinLogging.logger { }
}
override fun handles(url: GlideUrl): Boolean {
return true
}
override fun buildLoadData(glideUrl: GlideUrl, width: Int, height: Int, options: Options): LoadData<InputStream> {
log.warn { "Not using mirrors when loading $glideUrl" }
return LoadData(glideUrl, HttpFetcher(httpManager, glideUrl, proxyGetter()))
}
class Factory(
private val httpManager: HttpManager,
private val proxyGetter: () -> ProxyConfig?,
) : ModelLoaderFactory<GlideUrl, InputStream> {
override fun build(multiFactory: MultiModelLoaderFactory): ModelLoader<GlideUrl, InputStream> {
return HttpGlideUrlLoader(httpManager, proxyGetter)
}
override fun teardown() {}
}
}

View File

@ -27,6 +27,7 @@ import kotlin.test.Test
import kotlin.test.assertContentEquals
import kotlin.test.assertEquals
import kotlin.test.assertTrue
import kotlin.test.fail
private const val TOR_SOCKS_PORT = 9050
@ -71,6 +72,32 @@ class HttpDownloaderTest {
httpDownloader.download()
assertContentEquals(firstBytes + secondBytes, file.readBytes())
assertEquals(2, mockEngine.responseHistory.size)
}
@Test
fun testResumeError() = runSuspend {
val file = folder.newFile()
val firstBytes = Random.nextBytes(1024)
file.writeBytes(firstBytes)
val secondBytes = Random.nextBytes(1024)
val allBytes = firstBytes + secondBytes
var numRequest = 1
val mockEngine = MockEngine {
when (numRequest++) {
1 -> respond("", OK, headers = headersOf(ContentLength, "2048"))
2 -> respond(allBytes, OK) // not replying with PartialContent
3 -> respond(allBytes, OK)
else -> fail("Unexpected additional request")
}
}
val httpManager = HttpManager(userAgent, null, httpClientEngineFactory = get(mockEngine))
val httpDownloader = HttpDownloader(httpManager, downloadRequest, file)
httpDownloader.download()
assertContentEquals(allBytes, file.readBytes())
assertEquals(3, mockEngine.responseHistory.size)
}
@Test

View File

@ -4,10 +4,11 @@ import io.ktor.client.HttpClient
import io.ktor.client.call.receive
import io.ktor.client.engine.HttpClientEngineFactory
import io.ktor.client.engine.ProxyConfig
import io.ktor.client.features.HttpTimeout
import io.ktor.client.features.ResponseException
import io.ktor.client.features.ServerResponseException
import io.ktor.client.features.UserAgent
import io.ktor.client.features.defaultRequest
import io.ktor.client.features.timeout
import io.ktor.client.request.get
import io.ktor.client.request.head
import io.ktor.client.request.header
@ -35,6 +36,7 @@ import io.ktor.utils.io.core.toByteArray
import io.ktor.utils.io.readRemaining
import io.ktor.utils.io.writeFully
import mu.KotlinLogging
import kotlin.coroutines.cancellation.CancellationException
import kotlin.jvm.JvmOverloads
internal expect fun getHttpClientEngineFactory(): HttpClientEngineFactory<*>
@ -78,6 +80,7 @@ public open class HttpManager @JvmOverloads constructor(
install(UserAgent) {
agent = userAgent
}
install(HttpTimeout)
defaultRequest {
// add query string parameters if existing
parameters?.forEach { (key, value) ->
@ -93,7 +96,7 @@ public open class HttpManager @JvmOverloads constructor(
* This is useful for checking if the repository index has changed before downloading it again.
* However, due to non-standard ETags on mirrors, change detection is unreliable.
*/
suspend fun head(request: DownloadRequest, eTag: String? = null): HeadInfo? {
public suspend fun head(request: DownloadRequest, eTag: String? = null): HeadInfo? {
val authString = constructBasicAuthValue(request)
val response: HttpResponse = try {
mirrorChooser.mirrorRequest(request) { mirror, url ->
@ -102,6 +105,8 @@ public open class HttpManager @JvmOverloads constructor(
httpClient.head(url) {
// add authorization header from username / password if set
if (authString != null) header(Authorization, authString)
// increase connect timeout if using Tor mirror
if (mirror.isOnion()) timeout { connectTimeoutMillis = 10_000 }
}
}
} catch (e: ResponseException) {
@ -117,28 +122,20 @@ public open class HttpManager @JvmOverloads constructor(
}
@JvmOverloads
suspend fun get(
@Throws(ResponseException::class, NoResumeException::class, CancellationException::class)
public suspend fun get(
request: DownloadRequest,
skipFirstBytes: Long? = null,
receiver: suspend (ByteArray) -> Unit,
) {
val authString = constructBasicAuthValue(request)
mirrorChooser.mirrorRequest(request) { mirror, url ->
resetProxyIfNeeded(request.proxy, mirror)
log.debug { "GET $url" }
httpClient.get<HttpStatement>(url) {
// add authorization header from username / password if set
if (authString != null) header(Authorization, authString)
// add range header if set
if (skipFirstBytes != null) header(Range, "bytes=${skipFirstBytes}-")
}
}.execute { response ->
get(request, skipFirstBytes).execute { response ->
if (skipFirstBytes != null && response.status != PartialContent) {
throw ServerResponseException(response, "expected 206")
throw NoResumeException()
}
val channel: ByteReadChannel = response.receive()
val limit = 8L * 1024L
while (!channel.isClosedForRead) {
val packet = channel.readRemaining(8L * 1024L)
val packet = channel.readRemaining(limit)
while (!packet.isEmpty) {
receiver(packet.readBytes())
}
@ -146,12 +143,41 @@ public open class HttpManager @JvmOverloads constructor(
}
}
internal suspend fun get(
request: DownloadRequest,
skipFirstBytes: Long? = null,
): HttpStatement {
val authString = constructBasicAuthValue(request)
return mirrorChooser.mirrorRequest(request) { mirror, url ->
resetProxyIfNeeded(request.proxy, mirror)
log.debug { "GET $url" }
httpClient.get(url) {
// add authorization header from username / password if set
if (authString != null) header(Authorization, authString)
// increase connect timeout if using Tor mirror
if (mirror.isOnion()) timeout { connectTimeoutMillis = 20_000 }
// add range header if set
if (skipFirstBytes != null) header(Range, "bytes=${skipFirstBytes}-")
}
}
}
/**
* Returns a [ByteChannel] for streaming download.
*/
internal suspend fun getChannel(
request: DownloadRequest,
skipFirstBytes: Long? = null,
): ByteReadChannel {
return get(request, skipFirstBytes).receive()
}
/**
* Same as [get], but returns all bytes.
* Use this only when you are sure that a response will be small.
* Thus, this is intentionally visible internally only.
* Does not use [getChannel] so, it gets the [NoResumeException] as in the public API.
*/
@JvmOverloads
internal suspend fun getBytes(request: DownloadRequest, skipFirstBytes: Long? = null): ByteArray {
val channel = ByteChannel()
get(request, skipFirstBytes) { bytes ->
@ -176,7 +202,7 @@ public open class HttpManager @JvmOverloads constructor(
null
} else proxyConfig
if (currentProxy != newProxy) {
log.info { "Switching proxy from [$currentProxy] to [$newProxy]"}
log.info { "Switching proxy from [$currentProxy] to [$newProxy]" }
httpClient.close()
httpClient = getNewHttpClient(newProxy)
}

View File

@ -0,0 +1,3 @@
package org.fdroid.download
public class NoResumeException : Exception()

View File

@ -132,11 +132,9 @@ class HttpManagerTest {
assertContentEquals(content.copyOfRange(skipBytes, content.size),
httpManager.getBytes(downloadRequest, skipBytes.toLong()))
// second request fails, because it responds with OK and full content
val exception = assertFailsWith<ServerResponseException> {
assertFailsWith<NoResumeException> {
httpManager.getBytes(downloadRequest, skipBytes.toLong())
}
val url = mockEngine.requestHistory.last().url
assertEquals("Server error($url: 200 OK. Text: \"expected 206\"", exception.message)
}
@Test