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 fc2d1b021..b8706492d 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 @@ -129,7 +129,14 @@ internal class IndexV1UpdaterTest : DbTest() { assertIs(result.e) // check that the DB transaction was rolled back and the DB wasn't changed - assertEquals(repo, repoDao.getRepository(repoId) ?: fail()) + // except for adding the error to the repo + val expectedRepo = repo.copy( + preferences = repo.preferences.copy( + errorCount = 1, + lastError = "Signing certificate does not match" + ), + ) + assertEquals(expectedRepo, repoDao.getRepository(repoId) ?: fail()) assertEquals(0, appDao.countApps()) assertEquals(0, versionDao.countAppVersions()) } diff --git a/libs/database/src/main/java/org/fdroid/database/Repository.kt b/libs/database/src/main/java/org/fdroid/database/Repository.kt index 5b26a6b46..26e2486f0 100644 --- a/libs/database/src/main/java/org/fdroid/database/Repository.kt +++ b/libs/database/src/main/java/org/fdroid/database/Repository.kt @@ -111,6 +111,7 @@ public data class Repository internal constructor( lastUpdated: Long, username: String? = null, password: String? = null, + lastError: String? = null, ) : this( repository = CoreRepository( repoId = repoId, @@ -132,6 +133,7 @@ public data class Repository internal constructor( lastUpdated = lastUpdated, username = username, password = password, + lastError = lastError, ) ) @@ -232,6 +234,8 @@ public data class Repository internal constructor( get() { return "https://fdroid.link/#$address?fingerprint=$fingerprint" } + public val errorCount: Int get() = preferences.errorCount + public val lastError: String? get() = preferences.lastError } // Dummy repo to use in Compose Previews and in tests diff --git a/libs/database/src/main/java/org/fdroid/database/RepositoryDao.kt b/libs/database/src/main/java/org/fdroid/database/RepositoryDao.kt index 816172f33..f82bfba35 100644 --- a/libs/database/src/main/java/org/fdroid/database/RepositoryDao.kt +++ b/libs/database/src/main/java/org/fdroid/database/RepositoryDao.kt @@ -442,6 +442,21 @@ internal interface RepositoryDaoInt : RepositoryDao { ) fun getArchiveRepoId(cert: String): Long? + @Query( + """UPDATE ${RepositoryPreferences.TABLE} + SET errorCount = errorCount + 1, lastError = :errorMsg + WHERE repoId = :repoId + """ + ) + fun trackRepoUpdateError(repoId: Long, errorMsg: String) + + @Query( + """UPDATE ${RepositoryPreferences.TABLE} + SET errorCount = 0, lastError = NULL WHERE repoId = :repoId + """ + ) + fun resetRepoUpdateError(repoId: Long) + @Transaction override fun deleteRepository(repoId: Long) { deleteCoreRepository(repoId) 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 506340bb9..94ad6cb5a 100644 --- a/libs/database/src/main/java/org/fdroid/index/IndexUpdater.kt +++ b/libs/database/src/main/java/org/fdroid/index/IndexUpdater.kt @@ -1,7 +1,10 @@ package org.fdroid.index import android.net.Uri +import androidx.annotation.VisibleForTesting +import androidx.annotation.WorkerThread import org.fdroid.database.Repository +import org.fdroid.database.RepositoryDaoInt import org.fdroid.download.Downloader import org.fdroid.download.NotFoundException import java.io.File @@ -48,6 +51,8 @@ public fun interface TempFileProvider { * A class to update information of a [Repository] in the database with a new downloaded index. */ public abstract class IndexUpdater { + @VisibleForTesting + internal abstract val repoDao: RepositoryDaoInt /** * The [IndexFormatVersion] used by this updater. @@ -59,21 +64,46 @@ public abstract class IndexUpdater { /** * Updates an existing [repo] with a known [Repository.certificate]. */ - public fun update(repo: Repository): IndexUpdateResult = catchExceptions { - updateRepo(repo) + @WorkerThread + public fun update(repo: Repository): IndexUpdateResult = catchExceptions(repo) { + updateRepo(repo).also { result -> + // reset repo errors if repo updated fine again, but is still unchanged + if (repo.errorCount > 0 && result is IndexUpdateResult.Unchanged) { + repoDao.resetRepoUpdateError(repo.repoId) + } + } } - private fun catchExceptions(block: () -> IndexUpdateResult): IndexUpdateResult { + @WorkerThread + protected abstract fun updateRepo(repo: Repository): IndexUpdateResult + + @WorkerThread + private fun catchExceptions( + repo: Repository, + block: () -> IndexUpdateResult, + ): IndexUpdateResult { return try { block() } catch (e: NotFoundException) { + onError(repo.repoId, e) IndexUpdateResult.NotFound } catch (e: Exception) { + onError(repo.repoId, e) IndexUpdateResult.Error(e) } } - protected abstract fun updateRepo(repo: Repository): IndexUpdateResult + @WorkerThread + private fun onError(repoId: Long, e: Exception) { + val msg = buildString { + append(e.localizedMessage ?: e.message ?: e.javaClass.simpleName) + e.cause?.let { cause -> + append("\n") + append(cause.localizedMessage ?: cause.message ?: cause.javaClass.simpleName) + } + } + repoDao.trackRepoUpdateError(repoId, msg) + } } internal fun Downloader.setIndexUpdateListener( 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 505022730..a57dc0c36 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 @@ -8,6 +8,7 @@ import org.fdroid.database.DbV1StreamReceiver import org.fdroid.database.FDroidDatabase import org.fdroid.database.FDroidDatabaseInt import org.fdroid.database.Repository +import org.fdroid.database.RepositoryDaoInt import org.fdroid.download.DownloaderFactory import org.fdroid.index.IndexFormatVersion import org.fdroid.index.IndexFormatVersion.ONE @@ -34,6 +35,7 @@ public class IndexV1Updater( private val log = KotlinLogging.logger {} public override val formatVersion: IndexFormatVersion = ONE private val db: FDroidDatabaseInt = database as FDroidDatabaseInt + override val repoDao: RepositoryDaoInt = db.getRepositoryDao() override fun updateRepo(repo: Repository): IndexUpdateResult { // Normally, we shouldn't allow repository downgrades and assert the condition below. @@ -66,10 +68,11 @@ public class IndexV1Updater( streamProcessor.process(inputStream) } // update RepositoryPreferences with timestamp and ETag (for v1) - val repoDao = db.getRepositoryDao() val updatedPrefs = repo.preferences.copy( lastUpdated = System.currentTimeMillis(), lastETag = eTag, + errorCount = 0, + lastError = null, ) repoDao.updateRepositoryPreferences(updatedPrefs) } 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 87d10f022..5cd2ea3f3 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 @@ -6,6 +6,7 @@ import org.fdroid.database.DbV2StreamReceiver import org.fdroid.database.FDroidDatabase import org.fdroid.database.FDroidDatabaseInt import org.fdroid.database.Repository +import org.fdroid.database.RepositoryDaoInt import org.fdroid.download.DownloaderFactory import org.fdroid.index.IndexFormatVersion import org.fdroid.index.IndexFormatVersion.ONE @@ -33,6 +34,7 @@ public class IndexV2Updater( public override val formatVersion: IndexFormatVersion = TWO private val db: FDroidDatabaseInt = database as FDroidDatabaseInt + override val repoDao: RepositoryDaoInt = db.getRepositoryDao() override fun updateRepo(repo: Repository): IndexUpdateResult { val (_, entry) = getCertAndEntry(repo, repo.certificate) @@ -92,7 +94,6 @@ public class IndexV2Updater( try { downloader.download() file.inputStream().use { inputStream -> - val repoDao = db.getRepositoryDao() db.runInTransaction { // ensure somebody else hasn't updated the repo in the meantime val currentTimestamp = repoDao.getRepository(repo.repoId)?.timestamp @@ -108,6 +109,8 @@ public class IndexV2Updater( ?: error("No repo prefs for ${repo.repoId}") val updatedPrefs = repoPrefs.copy( lastUpdated = System.currentTimeMillis(), + errorCount = 0, + lastError = null, ) repoDao.updateRepositoryPreferences(updatedPrefs) } 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 8f547c042..86c5e617d 100644 --- a/libs/database/src/test/java/org/fdroid/repo/RepoAdderTest.kt +++ b/libs/database/src/test/java/org/fdroid/repo/RepoAdderTest.kt @@ -190,6 +190,8 @@ internal class RepoAdderTest { assertEquals(expectedResult, fetching.fetchResult) every { newRepo.formatVersion } returns IndexFormatVersion.TWO + every { newRepo.repoId } returns 1 + every { repoDao.trackRepoUpdateError(1, any()) } just Runs repoAdder.addFetchedRepository() @@ -225,6 +227,8 @@ internal class RepoAdderTest { assertIs(fetching.fetchResult) every { newRepo.formatVersion } returns IndexFormatVersion.TWO + every { newRepo.repoId } returns 1 + every { repoDao.trackRepoUpdateError(1, any()) } just Runs repoAdder.addFetchedRepository() @@ -852,6 +856,8 @@ internal class RepoAdderTest { assertIs(awaitItem()) // still Fetching from last call every { newRepo.formatVersion } returns IndexFormatVersion.TWO + every { newRepo.repoId } returns 1 + every { repoDao.trackRepoUpdateError(1, any()) } just Runs repoAdder.addFetchedRepository() assertIs(awaitItem()) // now moved to Adding