From 5ab649f14d859a833cd696df0e1822857487f2ae Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Tue, 14 Oct 2025 16:10:44 -0300 Subject: [PATCH] [db] optimize adding repos We now hang on to the index file while streaming it for repo preview purposes. This avoids having to re-download that file and we can properly add the repo right away. This then allows us to bring the user to the list of apps in that repository without it being initially empty. --- .../org/fdroid/index/v1/IndexV1UpdaterTest.kt | 2 +- .../org/fdroid/index/v2/IndexV2UpdaterTest.kt | 6 +- .../java/org/fdroid/index/IndexUpdater.kt | 4 +- .../main/java/org/fdroid/index/RepoManager.kt | 15 ++++- .../main/java/org/fdroid/index/RepoUpdater.kt | 10 +++- .../org/fdroid/index/v1/IndexV1Updater.kt | 2 +- .../org/fdroid/index/v2/IndexV2Updater.kt | 4 +- .../main/java/org/fdroid/repo/RepoAdder.kt | 49 +++++++++++---- .../main/java/org/fdroid/repo/RepoFetcher.kt | 8 ++- .../java/org/fdroid/repo/RepoV1Fetcher.kt | 18 +++--- .../java/org/fdroid/repo/RepoV2Fetcher.kt | 22 ++++--- .../java/org/fdroid/repo/SavingInputStream.kt | 45 ++++++++++++++ .../fdroid/repo/RepoAdderIntegrationTest.kt | 34 +++++++++-- .../java/org/fdroid/repo/RepoAdderTest.kt | 59 +++++++++++++++---- 14 files changed, 219 insertions(+), 59 deletions(-) create mode 100644 libs/database/src/main/java/org/fdroid/repo/SavingInputStream.kt diff --git a/libs/database/src/dbTest/java/org/fdroid/index/v1/IndexV1UpdaterTest.kt b/libs/database/src/dbTest/java/org/fdroid/index/v1/IndexV1UpdaterTest.kt index 89ed3d0fb..fc2d1b021 100644 --- a/libs/database/src/dbTest/java/org/fdroid/index/v1/IndexV1UpdaterTest.kt +++ b/libs/database/src/dbTest/java/org/fdroid/index/v1/IndexV1UpdaterTest.kt @@ -194,7 +194,7 @@ internal class IndexV1UpdaterTest : DbTest() { assets.open(jar).use { inputStream -> jarFile.outputStream().use { inputStream.copyTo(it) } } - every { tempFileProvider.createTempFile() } returns jarFile + every { tempFileProvider.createTempFile(null) } returns jarFile every { downloaderFactory.createWithTryFirstMirror(repo, uri, indexFile, jarFile) } returns downloader diff --git a/libs/database/src/dbTest/java/org/fdroid/index/v2/IndexV2UpdaterTest.kt b/libs/database/src/dbTest/java/org/fdroid/index/v2/IndexV2UpdaterTest.kt index 38db9bd6b..ed8bcfea2 100644 --- a/libs/database/src/dbTest/java/org/fdroid/index/v2/IndexV2UpdaterTest.kt +++ b/libs/database/src/dbTest/java/org/fdroid/index/v2/IndexV2UpdaterTest.kt @@ -283,7 +283,9 @@ internal class IndexV2UpdaterTest : DbTest() { assets.open("diff-empty-min/23.json").use { inputStream -> indexFile.outputStream().use { inputStream.copyTo(it) } } - every { tempFileProvider.createTempFile() } returnsMany listOf(entryFile, indexFile) + every { + tempFileProvider.createTempFile(any()) + } returnsMany listOf(entryFile, indexFile) val result2 = indexUpdater.update(repo2) assertIs(result2) @@ -312,7 +314,7 @@ internal class IndexV2UpdaterTest : DbTest() { indexFile.outputStream().use { inputStream.copyTo(it) } } - every { tempFileProvider.createTempFile() } returnsMany listOf(entryFile, indexFile) + every { tempFileProvider.createTempFile(any()) } returnsMany listOf(entryFile, indexFile) every { downloaderFactory.createWithTryFirstMirror(repo, entryUri, entryFileV2, any()) } returns downloader diff --git a/libs/database/src/main/java/org/fdroid/index/IndexUpdater.kt b/libs/database/src/main/java/org/fdroid/index/IndexUpdater.kt index 1695b550e..506340bb9 100644 --- a/libs/database/src/main/java/org/fdroid/index/IndexUpdater.kt +++ b/libs/database/src/main/java/org/fdroid/index/IndexUpdater.kt @@ -16,7 +16,7 @@ public sealed class IndexUpdateResult { public object Unchanged : IndexUpdateResult() public object Processed : IndexUpdateResult() public object NotFound : IndexUpdateResult() - public class Error(public val e: Exception) : IndexUpdateResult() + public data class Error(public val e: Exception) : IndexUpdateResult() } public interface IndexUpdateListener { @@ -41,7 +41,7 @@ internal val defaultRepoUriBuilder = RepoUriBuilder { repo, pathElements -> public fun interface TempFileProvider { @Throws(IOException::class) - public fun createTempFile(): File + public fun createTempFile(sha256: String?): File } /** diff --git a/libs/database/src/main/java/org/fdroid/index/RepoManager.kt b/libs/database/src/main/java/org/fdroid/index/RepoManager.kt index 60537fd48..8a61fcfdc 100644 --- a/libs/database/src/main/java/org/fdroid/index/RepoManager.kt +++ b/libs/database/src/main/java/org/fdroid/index/RepoManager.kt @@ -15,6 +15,8 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.launch import kotlinx.coroutines.withContext +import org.fdroid.CompatibilityChecker +import org.fdroid.CompatibilityCheckerImpl import org.fdroid.database.AppPrefs import org.fdroid.database.AppPrefsDaoInt import org.fdroid.database.FDroidDatabase @@ -39,12 +41,20 @@ public class RepoManager @JvmOverloads constructor( downloaderFactory: DownloaderFactory, httpManager: HttpManager, repoUriBuilder: RepoUriBuilder = defaultRepoUriBuilder, + compatibilityChecker: CompatibilityChecker = CompatibilityCheckerImpl( + packageManager = context.packageManager, + forceTouchApps = false, + ), private val coroutineContext: CoroutineContext = Dispatchers.IO, ) { private val repositoryDao = db.getRepositoryDao() as RepositoryDaoInt private val appPrefsDao = db.getAppPrefsDao() as AppPrefsDaoInt - private val tempFileProvider = TempFileProvider { - File.createTempFile("dl-", "", context.cacheDir) + private val tempFileProvider = TempFileProvider { sha256 -> + if (sha256 == null) { + File.createTempFile("dl-", "", context.cacheDir) + } else { + File(context.cacheDir, sha256).apply { createNewFile() } + } } private val repoAdder = RepoAdder( context = context, @@ -52,6 +62,7 @@ public class RepoManager @JvmOverloads constructor( tempFileProvider = tempFileProvider, downloaderFactory = downloaderFactory, httpManager = httpManager, + compatibilityChecker = compatibilityChecker, repoUriBuilder = repoUriBuilder, coroutineContext = coroutineContext, ) diff --git a/libs/database/src/main/java/org/fdroid/index/RepoUpdater.kt b/libs/database/src/main/java/org/fdroid/index/RepoUpdater.kt index 926824934..858f4493d 100644 --- a/libs/database/src/main/java/org/fdroid/index/RepoUpdater.kt +++ b/libs/database/src/main/java/org/fdroid/index/RepoUpdater.kt @@ -20,11 +20,15 @@ public class RepoUpdater( downloaderFactory: DownloaderFactory, repoUriBuilder: RepoUriBuilder = defaultRepoUriBuilder, compatibilityChecker: CompatibilityChecker, - listener: IndexUpdateListener, + listener: IndexUpdateListener? = null, ) { private val log = KotlinLogging.logger {} - private val tempFileProvider = TempFileProvider { - File.createTempFile("dl-", "", tempDir) + private val tempFileProvider = TempFileProvider { sha256 -> + if (sha256 == null) { + File.createTempFile("dl-", "", tempDir) + } else { + File(tempDir, sha256).apply { createNewFile() } + } } /** diff --git a/libs/database/src/main/java/org/fdroid/index/v1/IndexV1Updater.kt b/libs/database/src/main/java/org/fdroid/index/v1/IndexV1Updater.kt index ed4f822ff..505022730 100644 --- a/libs/database/src/main/java/org/fdroid/index/v1/IndexV1Updater.kt +++ b/libs/database/src/main/java/org/fdroid/index/v1/IndexV1Updater.kt @@ -42,7 +42,7 @@ public class IndexV1Updater( if (repo.formatVersion != null && repo.formatVersion != ONE) { log.error { "Format downgrade for ${repo.address}" } } - val file = tempFileProvider.createTempFile() + val file = tempFileProvider.createTempFile(null) val downloader = downloaderFactory.createWithTryFirstMirror( repo = repo, uri = repoUriBuilder.getUri(repo, SIGNED_FILE_NAME), diff --git a/libs/database/src/main/java/org/fdroid/index/v2/IndexV2Updater.kt b/libs/database/src/main/java/org/fdroid/index/v2/IndexV2Updater.kt index c17154084..87d10f022 100644 --- a/libs/database/src/main/java/org/fdroid/index/v2/IndexV2Updater.kt +++ b/libs/database/src/main/java/org/fdroid/index/v2/IndexV2Updater.kt @@ -54,7 +54,7 @@ public class IndexV2Updater( } private fun getCertAndEntry(repo: Repository, certificate: String): Pair { - val file = tempFileProvider.createTempFile() + val file = tempFileProvider.createTempFile(null) val downloader = downloaderFactory.createWithTryFirstMirror( repo = repo, uri = repoUriBuilder.getUri(repo, SIGNED_FILE_NAME), @@ -80,7 +80,7 @@ public class IndexV2Updater( repoVersion: Long, streamProcessor: IndexV2StreamProcessor, ): IndexUpdateResult { - val file = tempFileProvider.createTempFile() + val file = tempFileProvider.createTempFile(entryFile.sha256) val downloader = downloaderFactory.createWithTryFirstMirror( repo = repo, uri = repoUriBuilder.getUri(repo, entryFile.name.trimStart('/')), diff --git a/libs/database/src/main/java/org/fdroid/repo/RepoAdder.kt b/libs/database/src/main/java/org/fdroid/repo/RepoAdder.kt index 06d044d34..682f6164a 100644 --- a/libs/database/src/main/java/org/fdroid/repo/RepoAdder.kt +++ b/libs/database/src/main/java/org/fdroid/repo/RepoAdder.kt @@ -23,6 +23,7 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import kotlinx.serialization.SerializationException import mu.KotlinLogging +import org.fdroid.CompatibilityChecker import org.fdroid.database.AppOverviewItem import org.fdroid.database.FDroidDatabase import org.fdroid.database.MinimalApp @@ -34,6 +35,8 @@ import org.fdroid.download.HttpManager import org.fdroid.download.HttpManager.Companion.isInvalidHttpUrl import org.fdroid.download.NotFoundException import org.fdroid.index.IndexFormatVersion +import org.fdroid.index.IndexUpdateResult +import org.fdroid.index.RepoUpdater import org.fdroid.index.RepoUriBuilder import org.fdroid.index.SigningException import org.fdroid.index.TempFileProvider @@ -42,6 +45,7 @@ import org.fdroid.repo.AddRepoError.ErrorType.INVALID_INDEX import org.fdroid.repo.AddRepoError.ErrorType.IO_ERROR import org.fdroid.repo.AddRepoError.ErrorType.IS_ARCHIVE_REPO import org.fdroid.repo.AddRepoError.ErrorType.UNKNOWN_SOURCES_DISALLOWED +import java.io.File import java.io.IOException import java.net.Proxy import kotlin.coroutines.CoroutineContext @@ -57,11 +61,12 @@ public class Fetching( public val receivedRepo: Repository?, public val apps: List, public val fetchResult: FetchResult?, + public val indexFile: File? = null, +) : AddRepoState() { /** * true if fetching is complete. */ - public val done: Boolean = false, -) : AddRepoState() { + public val done: Boolean = indexFile != null override fun toString(): String { return "Fetching(fetchUrl=$fetchUrl, repo=${receivedRepo?.address}, apps=${apps.size}, " + "fetchResult=$fetchResult, done=$done)" @@ -72,6 +77,7 @@ public object Adding : AddRepoState() public class Added( public val repo: Repository, + public val updateResult: IndexUpdateResult?, ) : AddRepoState() public data class AddRepoError( @@ -103,6 +109,7 @@ internal class RepoAdder( private val tempFileProvider: TempFileProvider, private val downloaderFactory: DownloaderFactory, private val httpManager: HttpManager, + private val compatibilityChecker: CompatibilityChecker, private val repoUriGetter: RepoUriGetter = RepoUriGetter, private val repoUriBuilder: RepoUriBuilder = defaultRepoUriBuilder, private val coroutineContext: CoroutineContext = Dispatchers.IO, @@ -173,7 +180,7 @@ internal class RepoAdder( addRepoState.value = Fetching(fetchUrl, receivedRepo, apps, fetchResult) // try fetching repo with v2 format first and fallback to v1 - try { + val indexFile = try { fetchRepo(nUri.uri, nUri.fingerprint, proxy, nUri.username, nUri.password, receiver) } catch (e: SigningException) { log.error(e) { "Error verifying repo with given fingerprint." } @@ -197,7 +204,7 @@ internal class RepoAdder( if (finalRepo == null) { onError(AddRepoError(INVALID_INDEX)) } else { - addRepoState.value = Fetching(fetchUrl, finalRepo, apps, fetchResult, done = true) + addRepoState.value = Fetching(fetchUrl, finalRepo, apps, fetchResult, indexFile) } } @@ -207,6 +214,10 @@ internal class RepoAdder( } } + /** + * Fetches the repo from the given [uri] and posts updates to [receiver]. + * @return the temporary file the repo was written to. + */ private suspend fun fetchRepo( uri: Uri, fingerprint: String?, @@ -214,10 +225,9 @@ internal class RepoAdder( username: String?, password: String?, receiver: RepoPreviewReceiver, - ) { - try { - val repo = - getTempRepo(uri, IndexFormatVersion.TWO, username, password) + ): File { + return try { + val repo = getTempRepo(uri, IndexFormatVersion.TWO, username, password) val repoFetcher = RepoV2Fetcher( tempFileProvider, downloaderFactory, httpManager, repoUriBuilder, proxy ) @@ -225,8 +235,7 @@ internal class RepoAdder( } catch (e: NotFoundException) { log.warn(e) { "Did not find v2 repo, trying v1 now." } // try to fetch v1 repo - val repo = - getTempRepo(uri, IndexFormatVersion.ONE, username, password) + val repo = getTempRepo(uri, IndexFormatVersion.ONE, username, password) val repoFetcher = RepoV1Fetcher(tempFileProvider, downloaderFactory, repoUriBuilder) repoFetcher.fetchRepo(uri, repo, receiver, fingerprint) } @@ -270,7 +279,7 @@ internal class RepoAdder( } @WorkerThread - internal suspend fun addFetchedRepository(): Repository? { + internal fun addFetchedRepository(): Repository? { // prevent double calls (e.g. caused by double tapping a UI button) if (addRepoState.compareAndSet(Adding, Adding)) return null @@ -280,6 +289,7 @@ internal class RepoAdder( // get current state before changing it val state = (addRepoState.value as? Fetching) ?: throw IllegalStateException("Unexpected state: ${addRepoState.value}") + log.info { "Moved to state \'Adding\'..." } addRepoState.value = Adding val repo = state.receivedRepo @@ -287,6 +297,7 @@ internal class RepoAdder( val fetchResult = state.fetchResult ?: throw IllegalStateException("No fetchResult: ${addRepoState.value}") + var indexUpdateResult: IndexUpdateResult? = null val modifiedRepo: Repository = when (fetchResult) { is FetchResult.IsExistingRepository -> error("Repo exists: $fetchResult") is FetchResult.IsExistingMirror -> error("Mirror exists: $fetchResult") @@ -314,6 +325,17 @@ internal class RepoAdder( repositoryDao.updateUserMirrors(repoId, userMirrors) } repositoryDao.getRepository(repoId) ?: error("New repository not found in DB") + }.also { repo -> + // Update the repo before returning, so we already have its content. + // This should pick up [indexFile] automatically without re-downloading, + // because we use the sha256 hash as the file name. + indexUpdateResult = RepoUpdater( + tempDir = context.cacheDir, + db = db, + downloaderFactory = downloaderFactory, + compatibilityChecker = compatibilityChecker, + ).update(repo) + log.info { "Updated repo: $indexUpdateResult" } } } @@ -330,11 +352,14 @@ internal class RepoAdder( } } } - addRepoState.value = Added(modifiedRepo) + log.info { "Added repository" } + state.indexFile?.delete() + addRepoState.value = Added(modifiedRepo, indexUpdateResult) return modifiedRepo } internal fun abortAddingRepo() { + (addRepoState.value as? Fetching)?.indexFile?.delete() addRepoState.value = None fetchJob?.cancel() } diff --git a/libs/database/src/main/java/org/fdroid/repo/RepoFetcher.kt b/libs/database/src/main/java/org/fdroid/repo/RepoFetcher.kt index e8c244c02..eb47ae5e6 100644 --- a/libs/database/src/main/java/org/fdroid/repo/RepoFetcher.kt +++ b/libs/database/src/main/java/org/fdroid/repo/RepoFetcher.kt @@ -6,9 +6,15 @@ import org.fdroid.database.AppOverviewItem import org.fdroid.database.Repository import org.fdroid.download.NotFoundException import org.fdroid.index.SigningException +import java.io.File import java.io.IOException internal fun interface RepoFetcher { + /** + * Fetches the repo from the given [uri] and posts updates to [receiver]. + * @return the temporary file the repo was written to. + * Note that the OS may delete this at any time. + */ @Throws( IOException::class, SigningException::class, @@ -20,7 +26,7 @@ internal fun interface RepoFetcher { repo: Repository, receiver: RepoPreviewReceiver, fingerprint: String?, - ) + ): File } internal interface RepoPreviewReceiver { diff --git a/libs/database/src/main/java/org/fdroid/repo/RepoV1Fetcher.kt b/libs/database/src/main/java/org/fdroid/repo/RepoV1Fetcher.kt index ecd7fa8ef..7b5cf50b9 100644 --- a/libs/database/src/main/java/org/fdroid/repo/RepoV1Fetcher.kt +++ b/libs/database/src/main/java/org/fdroid/repo/RepoV1Fetcher.kt @@ -15,6 +15,7 @@ import org.fdroid.index.parseV1 import org.fdroid.index.v1.IndexV1Verifier import org.fdroid.index.v1.SIGNED_FILE_NAME import org.fdroid.index.v2.FileV2 +import java.io.File internal class RepoV1Fetcher( private val tempFileProvider: TempFileProvider, @@ -30,23 +31,19 @@ internal class RepoV1Fetcher( repo: Repository, receiver: RepoPreviewReceiver, fingerprint: String?, - ) { + ): File { // download and verify index-v1.jar - val indexFile = tempFileProvider.createTempFile() + val indexFile = tempFileProvider.createTempFile(null) val entryDownloader = downloaderFactory.create( repo = repo, uri = repoUriBuilder.getUri(repo, SIGNED_FILE_NAME), indexFile = FileV2.fromPath("/$SIGNED_FILE_NAME"), destFile = indexFile, ) - val (cert, indexV1) = try { - entryDownloader.download() - val verifier = IndexV1Verifier(indexFile, null, fingerprint) - verifier.getStreamAndVerify { inputStream -> - IndexParser.parseV1(inputStream) - } - } finally { - indexFile.delete() + entryDownloader.download() + val verifier = IndexV1Verifier(indexFile, null, fingerprint) + val (cert, indexV1) = verifier.getStreamAndVerify { inputStream -> + IndexParser.parseV1(inputStream) } val version = indexV1.repo.version val indexV2 = IndexConverter().toIndexV2(indexV1) @@ -63,5 +60,6 @@ internal class RepoV1Fetcher( val app = RepoV2StreamReceiver.getAppOverViewItem(packageName, packageV2, locales) receiver.onAppReceived(app) } + return indexFile } } diff --git a/libs/database/src/main/java/org/fdroid/repo/RepoV2Fetcher.kt b/libs/database/src/main/java/org/fdroid/repo/RepoV2Fetcher.kt index ed18010e7..eaf129e72 100644 --- a/libs/database/src/main/java/org/fdroid/repo/RepoV2Fetcher.kt +++ b/libs/database/src/main/java/org/fdroid/repo/RepoV2Fetcher.kt @@ -17,6 +17,7 @@ import org.fdroid.index.v2.EntryVerifier import org.fdroid.index.v2.FileV2 import org.fdroid.index.v2.IndexV2FullStreamProcessor import org.fdroid.index.v2.SIGNED_FILE_NAME +import java.io.File import java.net.Proxy import java.security.DigestInputStream import java.security.MessageDigest @@ -36,9 +37,9 @@ internal class RepoV2Fetcher( repo: Repository, receiver: RepoPreviewReceiver, fingerprint: String?, - ) { + ): File { // download and verify entry - val entryFile = tempFileProvider.createTempFile() + val entryFile = tempFileProvider.createTempFile(null) val entryDownloader = downloaderFactory.create( repo = repo, uri = repoUriBuilder.getUri(repo, SIGNED_FILE_NAME), @@ -59,7 +60,8 @@ internal class RepoV2Fetcher( val streamReceiver = RepoV2StreamReceiver(receiver, cert, repo.username, repo.password) val streamProcessor = IndexV2FullStreamProcessor(streamReceiver) - val digestInputStream = if (uri.scheme?.startsWith("http") == true) { + val indexFile = tempFileProvider.createTempFile(entry.index.sha256) + val inputStream = if (uri.scheme?.startsWith("http") == true) { // stream index for http(s) downloads val indexRequest = DownloadRequest( indexFile = entry.index, @@ -68,10 +70,11 @@ internal class RepoV2Fetcher( username = repo.username, password = repo.password, ) - httpManager.getDigestInputStream(indexRequest) + val digestInputStream = httpManager.getDigestInputStream(indexRequest) + // wrap stream to exfiltrate index file for later usage + SavingInputStream(digestInputStream, indexFile) } else { // no streaming supported, download file first - val indexFile = tempFileProvider.createTempFile() val indexDownloader = downloaderFactory.create( repo = repo, uri = repoUriBuilder.getUri(repo, entry.index.name.trimStart('/')), @@ -82,12 +85,17 @@ internal class RepoV2Fetcher( val digest = MessageDigest.getInstance("SHA-256") DigestInputStream(indexFile.inputStream(), digest) } - digestInputStream.use { inputStream -> + inputStream.use { inputStream -> streamProcessor.process(entry.version, inputStream) { } } - val hexDigest = digestInputStream.getDigestHex() + val hexDigest = when (inputStream) { + is DigestInputStream -> inputStream.getDigestHex() + is SavingInputStream -> inputStream.inputStream.getDigestHex() + else -> error("Unknown InputStream ${inputStream::class.java}") + } if (!hexDigest.equals(entry.index.sha256, ignoreCase = true)) { throw SigningException("Invalid ${entry.index.name} hash: $hexDigest") } + return indexFile } } diff --git a/libs/database/src/main/java/org/fdroid/repo/SavingInputStream.kt b/libs/database/src/main/java/org/fdroid/repo/SavingInputStream.kt new file mode 100644 index 000000000..d3d77038c --- /dev/null +++ b/libs/database/src/main/java/org/fdroid/repo/SavingInputStream.kt @@ -0,0 +1,45 @@ +package org.fdroid.repo + +import java.io.File +import java.io.InputStream +import java.security.DigestInputStream + +internal class SavingInputStream( + val inputStream: DigestInputStream, + outputFile: File, +) : InputStream() { + + private val outputStream = outputFile.outputStream() + + override fun read(): Int { + val byte = inputStream.read() + if (byte != -1) { + outputStream.write(byte) + } + return byte + } + + override fun read(b: ByteArray): Int { + val bytesRead = inputStream.read(b) + if (bytesRead != -1) { + outputStream.write(b, 0, bytesRead) + } + return bytesRead + } + + override fun read(b: ByteArray, off: Int, len: Int): Int { + val bytesRead = inputStream.read(b, off, len) + if (bytesRead != -1) { + outputStream.write(b, off, bytesRead) + } + return bytesRead + } + + override fun close() { + try { + inputStream.close() + } finally { + outputStream.close() + } + } +} diff --git a/libs/database/src/test/java/org/fdroid/repo/RepoAdderIntegrationTest.kt b/libs/database/src/test/java/org/fdroid/repo/RepoAdderIntegrationTest.kt index 478470eb8..06bb96b70 100644 --- a/libs/database/src/test/java/org/fdroid/repo/RepoAdderIntegrationTest.kt +++ b/libs/database/src/test/java/org/fdroid/repo/RepoAdderIntegrationTest.kt @@ -4,16 +4,21 @@ import androidx.core.os.LocaleListCompat import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry import app.cash.turbine.test +import io.mockk.MockKException import io.mockk.every import io.mockk.mockk +import io.mockk.slot import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest -import org.fdroid.database.FDroidDatabase +import org.fdroid.CompatibilityChecker +import org.fdroid.database.FDroidDatabaseInt import org.fdroid.database.NewRepository import org.fdroid.database.Repository import org.fdroid.database.RepositoryDaoInt import org.fdroid.download.HttpManager import org.fdroid.download.TestDownloadFactory +import org.fdroid.index.IndexFormatVersion +import org.fdroid.index.IndexUpdateResult import org.fdroid.index.TempFileProvider import org.fdroid.repo.AddRepoError.ErrorType.INVALID_FINGERPRINT import org.junit.Assume.assumeTrue @@ -22,7 +27,9 @@ import org.junit.Rule import org.junit.Test import org.junit.rules.TemporaryFolder import org.junit.runner.RunWith +import java.util.concurrent.Callable import kotlin.test.assertEquals +import kotlin.test.assertIs import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -34,17 +41,25 @@ internal class RepoAdderIntegrationTest { var folder: TemporaryFolder = TemporaryFolder() private val context = InstrumentationRegistry.getInstrumentation().targetContext - private val db = mockk() + private val db = mockk() private val repoDao = mockk() private val tempFileProvider = TempFileProvider { folder.newFile() } private val httpManager = HttpManager("test") private val downloaderFactory = TestDownloadFactory(httpManager) + private val compatibilityChecker = mockk() private val repoAdder: RepoAdder init { every { db.getRepositoryDao() } returns repoDao - repoAdder = RepoAdder(context, db, tempFileProvider, downloaderFactory, httpManager) + repoAdder = RepoAdder( + context = context, + db = db, + tempFileProvider = tempFileProvider, + downloaderFactory = downloaderFactory, + httpManager = httpManager, + compatibilityChecker = compatibilityChecker, + ) } @Before @@ -80,7 +95,9 @@ internal class RepoAdderIntegrationTest { assertEquals(1, (awaitItem() as Fetching).apps.size) assertEquals(2, (awaitItem() as Fetching).apps.size) assertEquals(3, (awaitItem() as Fetching).apps.size) - assertTrue(awaitItem() is Fetching) + assertEquals(4, (awaitItem() as Fetching).apps.size) + assertEquals(5, (awaitItem() as Fetching).apps.size) + assertTrue((awaitItem() as Fetching).done) } val state = repoAdder.addRepoState.value @@ -90,7 +107,12 @@ internal class RepoAdderIntegrationTest { println(" ${app.packageName} ${app.summary}") } + val runSlot = slot>() + every { db.runInTransaction(capture(runSlot)) } answers { + runSlot.captured.call() + } val newRepo: Repository = mockk() + every { newRepo.formatVersion } returns IndexFormatVersion.TWO every { repoDao.insert(any()) } returns 42L every { repoDao.getRepository(42L) } returns newRepo @@ -99,6 +121,10 @@ internal class RepoAdderIntegrationTest { val addedState = awaitItem() assertTrue(addedState is Added, addedState.toString()) assertEquals(newRepo, addedState.repo) + // we are not mocking all the actual repo adding, + // so just assert that this fails due to mocking + assertIs(addedState.updateResult) + assertIs(addedState.updateResult.e) } } diff --git a/libs/database/src/test/java/org/fdroid/repo/RepoAdderTest.kt b/libs/database/src/test/java/org/fdroid/repo/RepoAdderTest.kt index ce3422baa..415fa0259 100644 --- a/libs/database/src/test/java/org/fdroid/repo/RepoAdderTest.kt +++ b/libs/database/src/test/java/org/fdroid/repo/RepoAdderTest.kt @@ -9,6 +9,7 @@ import androidx.core.os.LocaleListCompat import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry import app.cash.turbine.test +import io.mockk.MockKException import io.mockk.Runs import io.mockk.coEvery import io.mockk.every @@ -21,8 +22,9 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.delay import kotlinx.coroutines.launch import kotlinx.coroutines.test.runTest +import org.fdroid.CompatibilityChecker import org.fdroid.LocaleChooser.getBestLocale -import org.fdroid.database.FDroidDatabase +import org.fdroid.database.FDroidDatabaseInt import org.fdroid.database.Mirror import org.fdroid.database.NewRepository import org.fdroid.database.Repository @@ -37,6 +39,7 @@ import org.fdroid.download.NotFoundException import org.fdroid.download.getDigestInputStream import org.fdroid.index.IndexFormatVersion import org.fdroid.index.IndexParser.json +import org.fdroid.index.IndexUpdateResult import org.fdroid.index.SigningException import org.fdroid.index.TempFileProvider import org.fdroid.index.v2.IndexV2 @@ -77,10 +80,11 @@ internal class RepoAdderTest { var folder: TemporaryFolder = TemporaryFolder() private val context = InstrumentationRegistry.getInstrumentation().targetContext - private val db = mockk() + private val db = mockk() private val repoDao = mockk() private val tempFileProvider = mockk() private val httpManager = mockk() + private val compatibilityChecker = mockk() private val downloaderFactory = mockk() private val downloader = mockk() private val digest = mockk() @@ -96,14 +100,28 @@ internal class RepoAdderTest { mockkStatic("org.fdroid.download.HttpManagerKt") - repoAdder = RepoAdder(context, db, tempFileProvider, downloaderFactory, httpManager) + repoAdder = RepoAdder( + context = context, + db = db, + tempFileProvider = tempFileProvider, + downloaderFactory = downloaderFactory, + httpManager = httpManager, + compatibilityChecker = compatibilityChecker, + ) } @Test fun testDisallowInstallUnknownSources() = runTest { val context = mockk() val userManager = mockk() - val repoAdder = RepoAdder(context, db, tempFileProvider, downloaderFactory, httpManager) + val repoAdder = RepoAdder( + context, + db, + tempFileProvider, + downloaderFactory, + httpManager, + compatibilityChecker, + ) every { context.getSystemService(UserManager::class.java) } returns userManager every { @@ -170,6 +188,8 @@ internal class RepoAdderTest { val fetching: Fetching = awaitItem() as Fetching // still Fetching from last call assertEquals(expectedResult, fetching.fetchResult) + every { newRepo.formatVersion } returns IndexFormatVersion.TWO + repoAdder.addFetchedRepository() assertIs(awaitItem()) // now moved to Adding @@ -177,6 +197,10 @@ internal class RepoAdderTest { val addedState = awaitItem() assertIs(addedState) assertEquals(newRepo, addedState.repo) + // we are not mocking all the actual repo adding, + // so just assert that this fails due to mocking + assertIs(addedState.updateResult) + assertIs(addedState.updateResult.e) } } @@ -199,6 +223,8 @@ internal class RepoAdderTest { val fetching: Fetching = awaitItem() as Fetching // still Fetching from last call assertIs(fetching.fetchResult) + every { newRepo.formatVersion } returns IndexFormatVersion.TWO + repoAdder.addFetchedRepository() assertIs(awaitItem()) // now moved to Adding @@ -206,6 +232,10 @@ internal class RepoAdderTest { val addedState = awaitItem() assertIs(addedState) assertEquals(newRepo, addedState.repo) + // we are not mocking all the actual repo adding, + // so just assert that this fails due to mocking + assertIs(addedState.updateResult) + assertIs(addedState.updateResult.e) } verify(exactly = 0) { @@ -409,7 +439,7 @@ internal class RepoAdderTest { val url = "https://example.org/repo" val jarFile = folder.newFile() - every { tempFileProvider.createTempFile() } returns jarFile + every { tempFileProvider.createTempFile(any()) } returns jarFile every { downloaderFactory.create( repo = match { it.address == url && it.formatVersion == IndexFormatVersion.TWO }, @@ -445,7 +475,7 @@ internal class RepoAdderTest { val index = "{ invalid JSON foo bar,".toByteArray() val indexStream = DigestInputStream(ByteArrayInputStream(index), digest) - every { tempFileProvider.createTempFile() } returns jarFile + every { tempFileProvider.createTempFile(any()) } returns jarFile every { downloaderFactory.create( repo = match { @@ -496,7 +526,7 @@ internal class RepoAdderTest { val url = "https://example.org/repo/" val jarFile = folder.newFile() - every { tempFileProvider.createTempFile() } returns jarFile + every { tempFileProvider.createTempFile(any()) } returns jarFile every { downloaderFactory.create( repo = match { @@ -563,7 +593,7 @@ internal class RepoAdderTest { val index = json.encodeToString(IndexV2.serializer(), indexV2).toByteArray() val indexStream = DigestInputStream(ByteArrayInputStream(index), digest) - every { tempFileProvider.createTempFile() } returns jarFile + every { tempFileProvider.createTempFile(any()) } returns jarFile every { downloaderFactory.create( repo = match { @@ -623,7 +653,7 @@ internal class RepoAdderTest { val index = json.encodeToString(IndexV2.serializer(), indexV2).toByteArray() val indexStream = DigestInputStream(ByteArrayInputStream(index), digest) - every { tempFileProvider.createTempFile() } returns jarFile + every { tempFileProvider.createTempFile(any()) } returns jarFile every { downloaderFactory.create( repo = match { @@ -686,7 +716,7 @@ internal class RepoAdderTest { val urlTrimmed = "http://testy.at.or.at/fdroid/repo" val jarFile = folder.newFile() - every { tempFileProvider.createTempFile() } returns jarFile + every { tempFileProvider.createTempFile(any()) } returns jarFile every { downloaderFactory.create( @@ -743,7 +773,7 @@ internal class RepoAdderTest { val url = "https://example.org/repo" val jarFile = folder.newFile() - every { tempFileProvider.createTempFile() } returns jarFile + every { tempFileProvider.createTempFile(any()) } returns jarFile every { downloaderFactory.create( repo = match { it.address == url && it.formatVersion == IndexFormatVersion.TWO }, @@ -820,6 +850,7 @@ internal class RepoAdderTest { repoAdder.addRepoState.test { assertIs(awaitItem()) // still Fetching from last call + every { newRepo.formatVersion } returns IndexFormatVersion.TWO repoAdder.addFetchedRepository() assertIs(awaitItem()) // now moved to Adding @@ -827,6 +858,10 @@ internal class RepoAdderTest { val addedState = awaitItem() assertIs(addedState) assertEquals(newRepo, addedState.repo) + // we are not mocking all the actual repo adding, + // so just assert that this fails due to mocking + assertIs(addedState.updateResult) + assertIs(addedState.updateResult.e) } } @@ -867,7 +902,7 @@ internal class RepoAdderTest { val indexInputStream = assets.open(indexFile) val indexDigestStream = DigestInputStream(indexInputStream, digest) - every { tempFileProvider.createTempFile() } returns jarFile + every { tempFileProvider.createTempFile(any()) } returns jarFile every { downloaderFactory.create( repo = match {