Merge branch '3011-repoupdatemanager-tests' into 'master'

Add tests for RepoUpdateManager

Closes #3011

See merge request fdroid/fdroidclient!1566

app/build.gradle

@@ -228,6 +228,7 @@ dependencies {
     testImplementation libs.robolectric
     testImplementation libs.mockk
     testImplementation libs.mockito.core
+    testImplementation libs.turbine
     testImplementation libs.hamcrest
     testImplementation libs.slf4j.simple
 

app/src/main/java/org/fdroid/fdroid/RepoUpdateManager.kt

@@ -10,6 +10,7 @@ import androidx.lifecycle.asLiveData
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.asStateFlow
 import kotlinx.coroutines.flow.map
+import org.fdroid.CompatibilityChecker
 import org.fdroid.CompatibilityCheckerImpl
 import org.fdroid.database.FDroidDatabase
 import org.fdroid.database.Repository
@@ -33,8 +34,80 @@ class RepoUpdateManager @JvmOverloads constructor(
     private val db: FDroidDatabase,
     private val repoManager: RepoManager = FDroidApp.getRepoManager(context),
     private val notificationManager: NotificationManager = NotificationManager(context),
-    forceIndexV1: Boolean = Preferences.get().isForceOldIndexEnabled,
-) : IndexUpdateListener {
+    private val compatibilityChecker: CompatibilityChecker = CompatibilityCheckerImpl(
+        packageManager = context.packageManager,
+        forceTouchApps = Preferences.get().forceTouchApps(),
+    ),
+    private val indexUpdateListener: IndexUpdateListener = object : IndexUpdateListener {
+        override fun onDownloadProgress(repo: Repository, bytesRead: Long, totalBytes: Long) {
+            Log.d(TAG, "Downloading ${repo.address} ($bytesRead/$totalBytes)")
+            if (!Preferences.get().isUpdateNotificationEnabled) return
+
+            val percent = if (totalBytes > 0) {
+                Utils.getPercent(bytesRead, totalBytes)
+            } else {
+                -1
+            }
+            val size = Utils.getFriendlySize(bytesRead)
+            val message: String = if (totalBytes == -1L) {
+                context.getString(R.string.status_download_unknown_size, repo.address, size)
+            } else {
+                val totalSize = Utils.getFriendlySize(totalBytes)
+                context.getString(R.string.status_download, repo.address, size, totalSize, percent)
+            }
+            notificationManager.showUpdateRepoNotification(msg = message, progress = percent)
+        }
+
+        /**
+         * If an updater is unable to know how many apps it has to process (i.e. it
+         * is streaming apps to the database or performing a large database query
+         * which touches all apps, but is unable to report progress), then it call
+         * this listener with [totalApps] = 0. Doing so will result in a message of
+         * "Saving app details" sent to the user. If you know how many apps you have
+         * processed, then a message of "Saving app details (x/total)" is displayed.
+         */
+        override fun onUpdateProgress(repo: Repository, appsProcessed: Int, totalApps: Int) {
+            Log.d(TAG, "Committing ${repo.address} ($appsProcessed/$totalApps)")
+            if (!Preferences.get().isUpdateNotificationEnabled) return
+
+            if (totalApps > 0) notificationManager.showUpdateRepoNotification(
+                msg = context.getString(
+                    R.string.status_inserting_x_apps,
+                    appsProcessed,
+                    totalApps,
+                    repo.address,
+                ),
+                progress = Utils.getPercent(appsProcessed.toLong(), totalApps.toLong())
+            ) else notificationManager.showUpdateRepoNotification(
+                msg = context.getString(R.string.status_inserting_apps),
+            )
+        }
+    },
+    private val repoUpdater: RepoUpdater = RepoUpdater(
+        tempDir = context.cacheDir,
+        db = DBHelper.getDb(context),
+        downloaderFactory = DownloaderFactory.INSTANCE,
+        repoUriBuilder = RepoUriBuilder { repository, pathElements ->
+            val repoAddress = Utils.getRepoAddress(repository)
+            Utils.getUri(repoAddress, *pathElements)
+        },
+        compatibilityChecker = compatibilityChecker,
+        listener = indexUpdateListener,
+    ),
+    private val indexV1Updater: IndexV1Updater? = if (Preferences.get().isForceOldIndexEnabled) {
+        IndexV1Updater(
+            database = db,
+            tempFileProvider = { File.createTempFile("dl-", "", context.cacheDir) },
+            downloaderFactory = DownloaderFactory.INSTANCE,
+            repoUriBuilder = RepoUriBuilder { repository, pathElements ->
+                val repoAddress = Utils.getRepoAddress(repository)
+                Utils.getUri(repoAddress, *pathElements)
+            },
+            compatibilityChecker = compatibilityChecker,
+            listener = indexUpdateListener,
+        )
+    } else null,
+) {
 
     private val _isUpdating = MutableStateFlow(false)
     val isUpdating = _isUpdating.asStateFlow()
@@ -49,30 +122,6 @@ class RepoUpdateManager @JvmOverloads constructor(
     }
     val nextUpdateLiveData = nextUpdateFlow.asLiveData()
 
-    private val uriBuilder = RepoUriBuilder { repository, pathElements ->
-        val repoAddress = Utils.getRepoAddress(repository)
-        Utils.getUri(repoAddress, *pathElements)
-    }
-    private val compatibilityChecker = CompatibilityCheckerImpl(
-        packageManager = context.packageManager,
-        forceTouchApps = Preferences.get().forceTouchApps(),
-    )
-    private val repoUpdater: RepoUpdater = RepoUpdater(
-        tempDir = context.cacheDir,
-        db = DBHelper.getDb(context),
-        downloaderFactory = DownloaderFactory.INSTANCE,
-        repoUriBuilder = uriBuilder,
-        compatibilityChecker = compatibilityChecker,
-        listener = this@RepoUpdateManager,
-    )
-    private val indexV1Updater: IndexV1Updater? = if (forceIndexV1) IndexV1Updater(
-        database = db,
-        tempFileProvider = { File.createTempFile("dl-", "", context.cacheDir) },
-        downloaderFactory = DownloaderFactory.INSTANCE,
-        repoUriBuilder = uriBuilder,
-        compatibilityChecker = compatibilityChecker,
-        listener = this,
-    ) else null
     private val fdroidPrefs = Preferences.get()
 
     @WorkerThread
@@ -167,48 +216,4 @@ class RepoUpdateManager @JvmOverloads constructor(
             Utils.showToastFromService(context, msgBuilder.toString(), LENGTH_LONG)
         }
     }
-
-    override fun onDownloadProgress(repo: Repository, bytesRead: Long, totalBytes: Long) {
-        Log.d(TAG, "Downloading ${repo.address} ($bytesRead/$totalBytes)")
-        if (!fdroidPrefs.isUpdateNotificationEnabled) return
-
-        val percent = if (totalBytes > 0) {
-            Utils.getPercent(bytesRead, totalBytes)
-        } else {
-            -1
-        }
-        val size = Utils.getFriendlySize(bytesRead)
-        val message: String = if (totalBytes == -1L) {
-            context.getString(R.string.status_download_unknown_size, repo.address, size)
-        } else {
-            val totalSize = Utils.getFriendlySize(totalBytes)
-            context.getString(R.string.status_download, repo.address, size, totalSize, percent)
-        }
-        notificationManager.showUpdateRepoNotification(msg = message, progress = percent)
-    }
-
-    /**
-     * If an updater is unable to know how many apps it has to process (i.e. it
-     * is streaming apps to the database or performing a large database query
-     * which touches all apps, but is unable to report progress), then it call
-     * this listener with [totalApps] = 0. Doing so will result in a message of
-     * "Saving app details" sent to the user. If you know how many apps you have
-     * processed, then a message of "Saving app details (x/total)" is displayed.
-     */
-    override fun onUpdateProgress(repo: Repository, appsProcessed: Int, totalApps: Int) {
-        Log.d(TAG, "Committing ${repo.address} ($appsProcessed/$totalApps)")
-        if (!fdroidPrefs.isUpdateNotificationEnabled) return
-
-        if (totalApps > 0) notificationManager.showUpdateRepoNotification(
-            msg = context.getString(
-                R.string.status_inserting_x_apps,
-                appsProcessed,
-                totalApps,
-                repo.address,
-            ),
-            progress = Utils.getPercent(appsProcessed.toLong(), totalApps.toLong())
-        ) else notificationManager.showUpdateRepoNotification(
-            msg = context.getString(R.string.status_inserting_apps),
-        )
-    }
 }

app/src/test/java/org/fdroid/fdroid/RepoUpdateManagerTest.kt

@@ -0,0 +1,212 @@
+package org.fdroid.fdroid
+
+import android.content.Context
+import android.content.pm.PackageManager
+import android.os.Looper
+import app.cash.turbine.test
+import io.mockk.Runs
+import io.mockk.every
+import io.mockk.just
+import io.mockk.mockk
+import io.mockk.mockkObject
+import io.mockk.mockkStatic
+import io.mockk.verify
+import junit.framework.TestCase.assertEquals
+import junit.framework.TestCase.assertFalse
+import kotlinx.coroutines.runBlocking
+import org.fdroid.CompatibilityChecker
+import org.fdroid.database.FDroidDatabase
+import org.fdroid.database.Repository
+import org.fdroid.database.RepositoryDao
+import org.fdroid.fdroid.work.RepoUpdateWorker
+import org.fdroid.index.IndexUpdateResult
+import org.fdroid.index.RepoManager
+import org.fdroid.index.RepoUpdater
+import org.fdroid.index.v1.IndexV1Updater
+import org.junit.Assert.assertThrows
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import java.io.IOException
+
+class RepoUpdateManagerTest {
+
+    private val context: Context = mockk()
+    private val db: FDroidDatabase = mockk()
+    private val repoManager: RepoManager = mockk()
+    private val notificationManager: NotificationManager = mockk()
+    private val compatibilityChecker: CompatibilityChecker = mockk()
+    private val repoUpdater: RepoUpdater = mockk()
+    private val indexV1Updater: IndexV1Updater = mockk()
+
+    private val packageManager: PackageManager = mockk()
+    private val preferences: Preferences = mockk()
+    private val repositoryDao: RepositoryDao = mockk()
+
+    init {
+        // needed for Flow#asLiveData()
+        mockkStatic(Looper::class)
+        every { Looper.getMainLooper() } returns mockk<Looper> {
+            every { thread } returns Thread.currentThread()
+        }
+        // avoids having to deal with WorkManager here
+        mockkObject(RepoUpdateWorker)
+        every { RepoUpdateWorker.getAutoUpdateWorkInfo(any()) } returns mockk()
+
+        mockkStatic(Preferences::get)
+        every { Preferences.get() } returns preferences
+
+        every { context.packageManager } returns packageManager
+        every { context.getString(any(), any()) } returns "foo bar"
+        every { db.getRepositoryDao() } returns repositoryDao
+    }
+
+    private val repoUpdateManager = RepoUpdateManager(
+        context = context,
+        db = db,
+        repoManager = repoManager,
+        notificationManager = notificationManager,
+        compatibilityChecker = compatibilityChecker,
+        repoUpdater = repoUpdater,
+        indexV1Updater = null,
+    )
+
+    @Test
+    fun testUpdateSingleRepo() {
+        val repo: Repository = mockk(relaxed = true)
+
+        every { repoManager.getRepository(1L) } returns repo
+        every { preferences.isUpdateNotificationEnabled } returns true
+        every { notificationManager.showUpdateRepoNotification(any(), false, null) } just Runs
+        every { repoUpdater.update(repo) } returns IndexUpdateResult.Processed
+        every { notificationManager.cancelUpdateRepoNotification() } just Runs
+        every { repositoryDao.walCheckpoint() } just Runs
+
+        runBlocking {
+            repoUpdateManager.isUpdating.test {
+                assertFalse(awaitItem()) // not updating
+                // do the update now
+                assertEquals(IndexUpdateResult.Processed, repoUpdateManager.updateRepo(1L))
+                assertTrue(awaitItem()) // now updating
+                assertFalse(awaitItem()) // at the end again not updating
+            }
+        }
+
+        verify {
+            notificationManager.cancelUpdateRepoNotification()
+            repositoryDao.walCheckpoint()
+        }
+    }
+
+    @Test
+    fun testUpdateSingleForcedV1Repo() {
+        val repoUpdateManager = RepoUpdateManager(
+            context = context,
+            db = db,
+            repoManager = repoManager,
+            notificationManager = notificationManager,
+            compatibilityChecker = compatibilityChecker,
+            repoUpdater = repoUpdater,
+            indexV1Updater = indexV1Updater,
+        )
+        val repo: Repository = mockk(relaxed = true)
+
+        every { repoManager.getRepository(1L) } returns repo
+        every { preferences.isUpdateNotificationEnabled } returns true
+        every { notificationManager.showUpdateRepoNotification(any(), false, null) } just Runs
+        every { indexV1Updater.update(repo) } returns IndexUpdateResult.Unchanged
+        every { notificationManager.cancelUpdateRepoNotification() } just Runs
+        every { repositoryDao.walCheckpoint() } just Runs
+
+        runBlocking {
+            repoUpdateManager.isUpdating.test {
+                assertFalse(awaitItem()) // not updating
+                // do the update now and expect unchanged result
+                assertEquals(IndexUpdateResult.Unchanged, repoUpdateManager.updateRepo(1L))
+                assertTrue(awaitItem()) // now updating
+                assertFalse(awaitItem()) // at the end again not updating
+            }
+        }
+
+        verify {
+            notificationManager.cancelUpdateRepoNotification()
+            repositoryDao.walCheckpoint()
+        }
+    }
+
+    @Test
+    fun testUpdateSingleRepoCleansUpException() {
+        val repo: Repository = mockk(relaxed = true)
+
+        every { repoManager.getRepository(1L) } returns repo
+        every { preferences.isUpdateNotificationEnabled } returns true
+        every { notificationManager.showUpdateRepoNotification(any(), false, null) } just Runs
+        every { repoUpdater.update(repo) } throws IOException()
+        every { notificationManager.cancelUpdateRepoNotification() } just Runs
+        every { repositoryDao.walCheckpoint() } just Runs
+
+        runBlocking {
+            repoUpdateManager.isUpdating.test {
+                assertFalse(awaitItem()) // not updating
+                // do the update now
+                assertThrows(IOException::class.java) {
+                    repoUpdateManager.updateRepo(1L)
+                }
+                assertTrue(awaitItem()) // now updating
+                assertFalse(awaitItem()) // at the end again not updating
+            }
+        }
+
+        verify {
+            notificationManager.cancelUpdateRepoNotification()
+            repositoryDao.walCheckpoint()
+        }
+    }
+
+    @Test
+    fun testUpdateReposDoesntDoQuickRecheck() {
+        // we did a check just now
+        every { preferences.lastUpdateCheck } returns System.currentTimeMillis() - 500
+
+        repoUpdateManager.updateRepos()
+    }
+
+    @Test
+    fun testUpdateThreeReposOneDisabled() {
+        val repo1: Repository = mockk(relaxed = true) {
+            every { enabled } returns true
+        }
+        val repo2: Repository = mockk(relaxed = true) {
+            every { enabled } returns false
+        }
+        val repo3: Repository = mockk(relaxed = true) {
+            every { enabled } returns true
+        }
+
+        every { preferences.lastUpdateCheck } returns 1337
+        every { repositoryDao.getRepositories() } returns listOf(repo1, repo2)
+        every { preferences.isUpdateNotificationEnabled } returns true
+        every { notificationManager.showUpdateRepoNotification(any(), false, null) } just Runs
+        every { repoUpdater.update(repo1) } returns IndexUpdateResult.Unchanged
+        every { repoUpdater.update(repo3) } returns IndexUpdateResult.Processed
+
+        every { notificationManager.cancelUpdateRepoNotification() } just Runs
+        every { repositoryDao.walCheckpoint() } just Runs
+        every { preferences.lastUpdateCheck = any() } just Runs
+
+        runBlocking {
+            repoUpdateManager.isUpdating.test {
+                assertFalse(awaitItem()) // not updating
+                repoUpdateManager.updateRepos()
+                assertTrue(awaitItem()) // now updating
+                assertFalse(awaitItem()) // at the end again not updating
+            }
+        }
+
+        verify(exactly = 1) {
+            notificationManager.cancelUpdateRepoNotification()
+            repositoryDao.walCheckpoint()
+        }
+        // repo2 is disabled and should not get updated
+        verify(exactly = 0) { repoUpdater.update(repo2) }
+    }
+}