From 63ea05459693306859ee15a5eabd6eb6ce48582f Mon Sep 17 00:00:00 2001 From: Daniel Frett Date: Mon, 23 Sep 2024 09:58:07 -0600 Subject: [PATCH 1/5] create a couple common tools for use with AttachmentRepository tests --- .../db/repository/AttachmentsRepositoryIT.kt | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/library/db/src/test/kotlin/org/cru/godtools/db/repository/AttachmentsRepositoryIT.kt b/library/db/src/test/kotlin/org/cru/godtools/db/repository/AttachmentsRepositoryIT.kt index f61134b914..84ff65a1b6 100644 --- a/library/db/src/test/kotlin/org/cru/godtools/db/repository/AttachmentsRepositoryIT.kt +++ b/library/db/src/test/kotlin/org/cru/godtools/db/repository/AttachmentsRepositoryIT.kt @@ -1,6 +1,7 @@ package org.cru.godtools.db.repository import app.cash.turbine.test +import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -8,6 +9,7 @@ import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest @@ -20,6 +22,12 @@ abstract class AttachmentsRepositoryIT { abstract val repository: AttachmentsRepository abstract val toolsRepository: ToolsRepository + private val tool = randomTool("tool") + private val tool2 = randomTool("tool2") + + @BeforeTest + fun createTools() = runBlocking { toolsRepository.storeToolsFromSync(setOf(tool, tool2)) } + @Test fun `findAttachment()`() = testScope.runTest { assertNull(repository.findAttachment(1)) @@ -121,8 +129,6 @@ abstract class AttachmentsRepositoryIT { // region storeInitialAttachments() @Test fun `storeInitialAttachments() - Don't replace already existing attachments`() = testScope.runTest { - val tool = randomTool("tool") - toolsRepository.storeToolsFromSync(setOf(tool)) val attachment = Attachment(tool = tool) { filename = "sync.bin" } repository.storeAttachmentsFromSync(tool, listOf(attachment)) @@ -137,8 +143,6 @@ abstract class AttachmentsRepositoryIT { // region storeAttachmentsFromSync() @Test fun `storeAttachmentsFromSync() - Update existing attachment`() = testScope.runTest { - val tool = randomTool("tool") - toolsRepository.storeToolsFromSync(setOf(tool)) val attachment = Attachment(tool = tool) { filename = "initial.ext" sha256 = "initial" @@ -161,8 +165,6 @@ abstract class AttachmentsRepositoryIT { @Test fun `storeAttachmentsFromSync() - Don't overwrite downloaded flag`() = testScope.runTest { - val tool = randomTool("tool") - toolsRepository.storeToolsFromSync(setOf(tool)) val attachment = Attachment(tool = tool) { filename = "file.ext" sha256 = "file" @@ -178,16 +180,14 @@ abstract class AttachmentsRepositoryIT { @Test fun `storeAttachmentsFromSync() - remove stale attachments`() = testScope.runTest { - val tool1 = randomTool("tool1") - val tool2 = randomTool("tool2") - val attachment = Attachment(tool = tool1) - val attachmentStale = Attachment(tool = tool1) - val attachmentNew = Attachment(tool = tool1) + val attachment = Attachment(tool = tool) + val attachmentStale = Attachment(tool = tool) + val attachmentNew = Attachment(tool = tool) val attachmentOtherTool = Attachment(tool = tool2) - toolsRepository.storeToolsFromSync(listOf(tool1, tool2)) + toolsRepository.storeToolsFromSync(listOf(tool, tool2)) repository.storeInitialAttachments(listOf(attachment, attachmentStale, attachmentOtherTool)) - repository.storeAttachmentsFromSync(tool1, listOf(attachment, attachmentNew)) + repository.storeAttachmentsFromSync(tool, listOf(attachment, attachmentNew)) assertNotNull(repository.getAttachments()) { assertEquals(3, it.size) assertEquals(setOf(attachment.id, attachmentNew.id, attachmentOtherTool.id), it.map { it.id }.toSet()) @@ -198,16 +198,13 @@ abstract class AttachmentsRepositoryIT { // region deleteAttachmentsFor() @Test fun `deleteAttachmentsFor()`() = testScope.runTest { - val tool1 = randomTool("tool1") - val tool2 = randomTool("tool2") - toolsRepository.storeToolsFromSync(setOf(tool1, tool2)) val attachments = listOf( - Attachment(tool = tool1), + Attachment(tool = tool), Attachment(tool = tool2) ) repository.storeInitialAttachments(attachments) - repository.deleteAttachmentsFor(tool1) + repository.deleteAttachmentsFor(tool) assertNotNull(repository.getAttachments()) { assertEquals(1, it.size) assertEquals(attachments[1].id, it[0].id) From 2137a1f4623b75639a177e1c4fd11ddef658e4ce Mon Sep 17 00:00:00 2001 From: Daniel Frett Date: Mon, 23 Sep 2024 10:05:55 -0600 Subject: [PATCH 2/5] create all the test Attachments associated with an actual Tool --- .../db/repository/AttachmentsRepositoryIT.kt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/library/db/src/test/kotlin/org/cru/godtools/db/repository/AttachmentsRepositoryIT.kt b/library/db/src/test/kotlin/org/cru/godtools/db/repository/AttachmentsRepositoryIT.kt index 84ff65a1b6..fe8fa6d3f9 100644 --- a/library/db/src/test/kotlin/org/cru/godtools/db/repository/AttachmentsRepositoryIT.kt +++ b/library/db/src/test/kotlin/org/cru/godtools/db/repository/AttachmentsRepositoryIT.kt @@ -32,7 +32,7 @@ abstract class AttachmentsRepositoryIT { fun `findAttachment()`() = testScope.runTest { assertNull(repository.findAttachment(1)) - val attachment = Attachment().apply { + val attachment = Attachment(tool = tool).apply { id = 1 filename = "test.ext" } @@ -42,7 +42,7 @@ abstract class AttachmentsRepositoryIT { @Test fun `findAttachmentFlow()`() = testScope.runTest { - val attachment = Attachment().apply { + val attachment = Attachment(tool = tool).apply { id = 1 filename = "test.ext" } @@ -59,12 +59,12 @@ abstract class AttachmentsRepositoryIT { fun `getAttachments()`() = testScope.runTest { repository.storeInitialAttachments( listOf( - Attachment().apply { + Attachment(tool = tool).apply { id = 1 filename = "name1.bin" isDownloaded = true }, - Attachment().apply { + Attachment(tool = tool).apply { id = 2 filename = "name2.bin" } @@ -88,7 +88,7 @@ abstract class AttachmentsRepositoryIT { runCurrent() expectMostRecentItem() - val attachment = Attachment().apply { + val attachment = Attachment(tool = tool).apply { id = 1 filename = "name1.bin" isDownloaded = false @@ -108,7 +108,7 @@ abstract class AttachmentsRepositoryIT { fun `updateAttachmentDownloaded()`() = testScope.runTest { repository.storeInitialAttachments( listOf( - Attachment().apply { + Attachment(tool = tool).apply { id = 1 isDownloaded = false } From 0c4e8551bcec7a50aec6af03f0dd7db13e86e7d1 Mon Sep 17 00:00:00 2001 From: Daniel Frett Date: Mon, 23 Sep 2024 10:17:42 -0600 Subject: [PATCH 3/5] Don't store attachments referencing a tool that doesn't exist --- .../db/room/repository/AttachmentsRoomRepository.kt | 7 +++++-- .../godtools/db/repository/AttachmentsRepositoryIT.kt | 9 +++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/library/db/src/main/kotlin/org/cru/godtools/db/room/repository/AttachmentsRoomRepository.kt b/library/db/src/main/kotlin/org/cru/godtools/db/room/repository/AttachmentsRoomRepository.kt index 24f548dc5f..f6f0456b4c 100644 --- a/library/db/src/main/kotlin/org/cru/godtools/db/room/repository/AttachmentsRoomRepository.kt +++ b/library/db/src/main/kotlin/org/cru/godtools/db/room/repository/AttachmentsRoomRepository.kt @@ -26,8 +26,11 @@ internal abstract class AttachmentsRoomRepository(private val db: GodToolsRoomDa override suspend fun updateAttachmentDownloaded(id: Long, isDownloaded: Boolean) = dao.updateAttachmentDownloaded(id, isDownloaded) - override suspend fun storeInitialAttachments(attachments: Collection) = - dao.insertOrIgnoreAttachments(attachments.map { AttachmentEntity(it) }) + @Transaction + override suspend fun storeInitialAttachments(attachments: Collection) { + val tools = db.toolsDao.getTools().mapTo(mutableSetOf()) { it.code } + dao.insertOrIgnoreAttachments(attachments.filter { it.toolCode in tools }.map { AttachmentEntity(it) }) + } // region Sync Methods @Transaction diff --git a/library/db/src/test/kotlin/org/cru/godtools/db/repository/AttachmentsRepositoryIT.kt b/library/db/src/test/kotlin/org/cru/godtools/db/repository/AttachmentsRepositoryIT.kt index fe8fa6d3f9..9d51fac507 100644 --- a/library/db/src/test/kotlin/org/cru/godtools/db/repository/AttachmentsRepositoryIT.kt +++ b/library/db/src/test/kotlin/org/cru/godtools/db/repository/AttachmentsRepositoryIT.kt @@ -138,6 +138,15 @@ abstract class AttachmentsRepositoryIT { assertEquals("sync.bin", it.filename) } } + + @Test + fun `storeInitialAttachments() - Ignore attachments for tools that don't exist`() = testScope.runTest { + val missingTool = randomTool("missing") + val attachment = Attachment(tool = missingTool) + + repository.storeInitialAttachments(listOf(attachment)) + assertNull(repository.findAttachment(attachment.id)) + } // endregion storeInitialAttachments() // region storeAttachmentsFromSync() From 767c524a1292987fd8efb08f57c827c0f5420af8 Mon Sep 17 00:00:00 2001 From: Daniel Frett Date: Mon, 23 Sep 2024 10:56:22 -0600 Subject: [PATCH 4/5] support custom apiAttachments for randomTask() --- .../main/kotlin/org/cru/godtools/init/content/task/Tasks.kt | 2 +- .../model/src/main/kotlin/org/cru/godtools/model/Tool.kt | 6 ++++-- .../src/test/kotlin/org/cru/godtools/model/ToolTest.kt | 2 +- .../org/cru/godtools/sync/repository/SyncRepository.kt | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/library/initial-content/src/main/kotlin/org/cru/godtools/init/content/task/Tasks.kt b/library/initial-content/src/main/kotlin/org/cru/godtools/init/content/task/Tasks.kt index 9443602523..621cead5de 100644 --- a/library/initial-content/src/main/kotlin/org/cru/godtools/init/content/task/Tasks.kt +++ b/library/initial-content/src/main/kotlin/org/cru/godtools/init/content/task/Tasks.kt @@ -89,7 +89,7 @@ internal class Tasks @Inject constructor( } suspend fun loadBundledAttachments(data: BundledData) { - attachmentsRepository.storeInitialAttachments(data.getTools().flatMap { it.attachments.orEmpty() }) + attachmentsRepository.storeInitialAttachments(data.getTools().flatMap { it.apiAttachments.orEmpty() }) } suspend fun loadBundledTranslations(data: BundledData) { diff --git a/library/model/src/main/kotlin/org/cru/godtools/model/Tool.kt b/library/model/src/main/kotlin/org/cru/godtools/model/Tool.kt index 135ca8b306..f2f942da8e 100644 --- a/library/model/src/main/kotlin/org/cru/godtools/model/Tool.kt +++ b/library/model/src/main/kotlin/org/cru/godtools/model/Tool.kt @@ -79,6 +79,8 @@ class Tool( defaultVariantCode: String? = null, @JsonApiId val apiId: Long? = null, + @JsonApiAttribute(JSON_ATTACHMENTS) + val apiAttachments: List? = null, @JsonApiAttribute(JSON_LATEST_TRANSLATIONS) val translations: List? = null, @JsonApiIgnore @@ -171,8 +173,6 @@ class Tool( @JsonApiAttribute(JSON_INITIAL_FAVORITES_PRIORITY) val initialFavoritesPriority: Int? = null - @JsonApiAttribute(JSON_ATTACHMENTS) - val attachments: List? = null @Suppress("SENSELESS_COMPARISON") val isValid @@ -266,6 +266,7 @@ fun randomTool( metatoolCode: String? = UUID.randomUUID().toString().takeIf { Random.nextBoolean() }, defaultVariantCode: String? = UUID.randomUUID().toString().takeIf { Random.nextBoolean() }, apiId: Long? = Random.nextLong().takeIf { Random.nextBoolean() }, + apiAttachments: List? = null, changedFieldsStr: String = setOf(ATTR_IS_FAVORITE).filter { Random.nextBoolean() }.joinToString(","), ) = Tool( code = code, @@ -289,5 +290,6 @@ fun randomTool( metatoolCode = metatoolCode, defaultVariantCode = defaultVariantCode, apiId = apiId, + apiAttachments = apiAttachments, changedFieldsStr = changedFieldsStr, ) diff --git a/library/model/src/test/kotlin/org/cru/godtools/model/ToolTest.kt b/library/model/src/test/kotlin/org/cru/godtools/model/ToolTest.kt index 56e5d7f9ca..d94a909b31 100644 --- a/library/model/src/test/kotlin/org/cru/godtools/model/ToolTest.kt +++ b/library/model/src/test/kotlin/org/cru/godtools/model/ToolTest.kt @@ -39,7 +39,7 @@ class ToolTest { assertEquals(Tool.DEFAULT_DEFAULT_LOCALE, tool.defaultLocale) assertEquals(10, tool.defaultOrder) assertEquals(1L, tool.apiId) - assertThat(tool.attachments, hasSize(3)) + assertThat(tool.apiAttachments, hasSize(3)) assertThat(tool.translations, hasSize(2)) } diff --git a/library/sync/src/main/kotlin/org/cru/godtools/sync/repository/SyncRepository.kt b/library/sync/src/main/kotlin/org/cru/godtools/sync/repository/SyncRepository.kt index 9050814665..49907d794e 100644 --- a/library/sync/src/main/kotlin/org/cru/godtools/sync/repository/SyncRepository.kt +++ b/library/sync/src/main/kotlin/org/cru/godtools/sync/repository/SyncRepository.kt @@ -64,7 +64,7 @@ internal class SyncRepository @Inject constructor( } } if (includes.include(Tool.JSON_ATTACHMENTS)) { - tool.attachments?.let { launch { attachmentsRepository.storeAttachmentsFromSync(tool, it) } } + tool.apiAttachments?.let { launch { attachmentsRepository.storeAttachmentsFromSync(tool, it) } } } val metatool = tool.metatool From 97f7d30b1017b0123323a13a76d164c280940e5f Mon Sep 17 00:00:00 2001 From: Daniel Frett Date: Mon, 23 Sep 2024 10:57:16 -0600 Subject: [PATCH 5/5] don't load bundled attachments if attachments already exist --- .../cru/godtools/init/content/task/Tasks.kt | 3 ++ .../godtools/init/content/task/TasksTest.kt | 38 ++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/library/initial-content/src/main/kotlin/org/cru/godtools/init/content/task/Tasks.kt b/library/initial-content/src/main/kotlin/org/cru/godtools/init/content/task/Tasks.kt index 621cead5de..1572643832 100644 --- a/library/initial-content/src/main/kotlin/org/cru/godtools/init/content/task/Tasks.kt +++ b/library/initial-content/src/main/kotlin/org/cru/godtools/init/content/task/Tasks.kt @@ -89,6 +89,9 @@ internal class Tasks @Inject constructor( } suspend fun loadBundledAttachments(data: BundledData) { + // short-circuit if we already have any attachments loaded + if (attachmentsRepository.getAttachments().isNotEmpty()) return + attachmentsRepository.storeInitialAttachments(data.getTools().flatMap { it.apiAttachments.orEmpty() }) } diff --git a/library/initial-content/src/test/kotlin/org/cru/godtools/init/content/task/TasksTest.kt b/library/initial-content/src/test/kotlin/org/cru/godtools/init/content/task/TasksTest.kt index 03e907bae1..5fb967ec68 100644 --- a/library/initial-content/src/test/kotlin/org/cru/godtools/init/content/task/TasksTest.kt +++ b/library/initial-content/src/test/kotlin/org/cru/godtools/init/content/task/TasksTest.kt @@ -10,17 +10,21 @@ import io.mockk.confirmVerified import io.mockk.every import io.mockk.just import io.mockk.mockk +import io.mockk.slot import java.io.ByteArrayInputStream import java.util.Locale import kotlin.test.Test +import kotlin.test.assertEquals import kotlinx.coroutines.test.runTest import org.ccci.gto.android.common.jsonapi.JsonApiConverter import org.ccci.gto.android.common.jsonapi.model.JsonApiObject import org.cru.godtools.base.Settings +import org.cru.godtools.db.repository.AttachmentsRepository import org.cru.godtools.db.repository.LastSyncTimeRepository import org.cru.godtools.db.repository.ToolsRepository import org.cru.godtools.db.repository.TranslationsRepository import org.cru.godtools.downloadmanager.GodToolsDownloadManager +import org.cru.godtools.model.Attachment import org.cru.godtools.model.Tool import org.cru.godtools.model.randomTool import org.cru.godtools.model.randomTranslation @@ -40,6 +44,9 @@ class TasksTest { private val settings: Settings = mockk { every { appLanguage } returns Locale("x") } + private val attachmentsRepository: AttachmentsRepository = mockk { + coEvery { getAttachments() } returns emptyList() + } private val toolsRepository: ToolsRepository = mockk(relaxUnitFun = true) { coEvery { getNormalTools() } returns emptyList() } @@ -47,7 +54,7 @@ class TasksTest { private val tasks = Tasks( context, - mockk(), + attachmentsRepository = attachmentsRepository, downloadManager, jsonApiConverter, languagesRepository = mockk(), @@ -101,4 +108,33 @@ class TasksTest { confirmVerified(toolsRepository) } // endregion initFavoriteTools() + + // region loadBundledAttachments() + private val storedAttachments = slot>() + + @Test + fun `loadBundledAttachments()`() = runTest { + val tools = Array(5) { + randomTool(type = Tool.Type.TRACT, apiId = it.toLong(), apiAttachments = List(5) { Attachment() }) + } + every { jsonApiConverter.fromJson(any(), Tool::class.java) } returns JsonApiObject.of(*tools) + coEvery { attachmentsRepository.storeInitialAttachments(capture(storedAttachments)) } just Runs + + tasks.loadBundledAttachments(tasks.bundledData()) + coVerifyAll { + attachmentsRepository.getAttachments() + attachmentsRepository.storeInitialAttachments(any()) + } + assertEquals(25, storedAttachments.captured.size) + } + + @Test + fun `loadBundledAttachments() - Already Ran - Has attachments`() = runTest { + val attachments = List(5) { Attachment() } + coEvery { attachmentsRepository.getAttachments() } returns attachments + + tasks.loadBundledAttachments(tasks.bundledData()) + coVerifyAll { attachmentsRepository.getAttachments() } + } + // endregion loadBundledAttachments() }