upflit(beta): fix: avatar image uri (#10500) (#10522)

Author: coreycb

feature/account/api/build.gradle.kts

@@ -11,11 +11,6 @@ kotlin {
         commonMain.dependencies {
             api(projects.core.architecture.api)
         }
-
-        androidMain.dependencies {
-            // ensure Android target can consume the module if it's platform-specific
-            api(projects.core.architecture.api)
-        }
     }
 }
 

feature/account/avatar/api/build.gradle.kts

@@ -8,7 +8,6 @@ kotlin {
     }
 
     sourceSets {
-
         commonMain.dependencies {
             api(projects.feature.account.api)
             api(libs.uri)

feature/account/avatar/impl/src/main/kotlin/net/thunderbird/feature/account/avatar/data/datasource/LocalAvatarImageDataSource.kt

@@ -1,6 +1,8 @@
 package net.thunderbird.feature.account.avatar.data.datasource
 
 import com.eygraber.uri.Uri
+import kotlin.time.Clock
+import kotlin.time.ExperimentalTime
 import net.thunderbird.core.file.DirectoryProvider
 import net.thunderbird.core.file.FileManager
 import net.thunderbird.feature.account.AccountId
@@ -15,17 +17,22 @@ import net.thunderbird.feature.account.avatar.data.AvatarDataContract.DataSource
  * @param fileManager The [FileManager] for file operations.
  * @param directoryProvider The [DirectoryProvider] to get app directories.
  */
+@OptIn(ExperimentalTime::class)
 internal class LocalAvatarImageDataSource(
     private val fileManager: FileManager,
     private val directoryProvider: DirectoryProvider,
+    private val clock: Clock,
 ) : LocalAvatarImage {
 
     override suspend fun update(id: AccountId, imageUri: Uri): Uri {
         val avatarImageUri = getAvatarImageUri(id)
 
         fileManager.copy(imageUri, avatarImageUri)
 
-        return avatarImageUri
+        return avatarImageUri.buildUpon()
+            .clearQuery()
+            .appendQueryParameter("v", clock.now().toEpochMilliseconds().toString())
+            .build()
     }
 
     override suspend fun delete(id: AccountId) {

feature/account/avatar/impl/src/main/kotlin/net/thunderbird/feature/account/avatar/di/FeatureAccountAvatarModule.kt

@@ -1,16 +1,19 @@
 package net.thunderbird.feature.account.avatar.di
 
+import kotlin.time.ExperimentalTime
 import net.thunderbird.feature.account.avatar.AvatarImageRepository
 import net.thunderbird.feature.account.avatar.data.AvatarDataContract.DataSource
 import net.thunderbird.feature.account.avatar.data.DefaultAvatarImageRepository
 import net.thunderbird.feature.account.avatar.data.datasource.LocalAvatarImageDataSource
 import org.koin.dsl.module
 
+@OptIn(ExperimentalTime::class)
 val featureAccountAvatarModule = module {
     single<DataSource.LocalAvatarImage> {
         LocalAvatarImageDataSource(
             fileManager = get(),
             directoryProvider = get(),
+            clock = get(),
         )
     }
 

feature/account/avatar/impl/src/test/kotlin/net/thunderbird/feature/account/avatar/data/datasource/LocalAvatarImageDataSourceTest.kt

@@ -2,18 +2,24 @@ package net.thunderbird.feature.account.avatar.data.datasource
 
 import assertk.assertThat
 import assertk.assertions.isEqualTo
+import assertk.assertions.isNotEqualTo
 import assertk.assertions.isNotNull
 import assertk.assertions.isNull
 import com.eygraber.uri.toKmpUri
 import kotlin.test.BeforeTest
 import kotlin.test.Test
+import kotlin.time.Duration.Companion.milliseconds
+import kotlin.time.ExperimentalTime
+import kotlin.time.Instant
 import kotlinx.coroutines.test.runTest
 import net.thunderbird.core.file.DirectoryProvider
+import net.thunderbird.core.testing.TestClock
 import net.thunderbird.feature.account.AccountIdFactory
 import net.thunderbird.feature.account.avatar.data.AvatarDataContract
 import org.junit.Rule
 import org.junit.rules.TemporaryFolder
 
+@OptIn(ExperimentalTime::class)
 class LocalAvatarImageDataSourceTest {
 
     @JvmField
@@ -22,37 +28,57 @@ class LocalAvatarImageDataSourceTest {
 
     private lateinit var directoryProvider: DirectoryProvider
     private lateinit var fileManager: CapturingFileManager
+    private lateinit var clock: TestClock
     private lateinit var testSubject: LocalAvatarImageDataSource
 
     @BeforeTest
     fun setUp() {
         val appDir = folder.newFolder("app")
         directoryProvider = FakeDirectoryProvider(appDir.absolutePath.toKmpUri())
         fileManager = CapturingFileManager()
-        testSubject = LocalAvatarImageDataSource(fileManager, directoryProvider)
+        clock = TestClock(Instant.fromEpochMilliseconds(1_000))
+        testSubject = LocalAvatarImageDataSource(fileManager, directoryProvider, clock)
     }
 
     @Test
-    fun `update should copy image to expected path and return destination uri`() = runTest {
+    fun `update should copy image to expected path and return versioned destination uri`() = runTest {
         // Arrange
         val accountId = AccountIdFactory.create()
         val source = "file:///external/picked/image.jpg".toKmpUri()
         val expectedDir = directoryProvider.getFilesDir().buildUpon()
             .appendPath(AvatarDataContract.DataSource.LocalAvatarImage.DIRECTORY_NAME)
             .build()
         val expectedDest = expectedDir.buildUpon().appendPath("$accountId.jpg").build()
+        val expectedVersioned = expectedDest.buildUpon().appendQueryParameter("v", "1000").build()
 
         // Act
         val returned = testSubject.update(accountId, source)
 
         // Assert
-        assertThat(returned).isEqualTo(expectedDest)
+        assertThat(returned).isEqualTo(expectedVersioned)
         assertThat(fileManager.lastCreatedDir).isEqualTo(expectedDir)
         assertThat(fileManager.lastCopySource).isEqualTo(source)
         assertThat(fileManager.lastCopyDestination).isEqualTo(expectedDest)
         assertThat(fileManager.lastDeleted).isNull()
     }
 
+    @Test
+    fun `successive updates should return different URIs`() = runTest {
+        // Arrange
+        val accountId = AccountIdFactory.create()
+        val source = "file:///external/picked/image.jpg".toKmpUri()
+
+        // Act
+        val uri1 = testSubject.update(accountId, source)
+        clock.advanceTimeBy(1.milliseconds)
+        val uri2 = testSubject.update(accountId, source)
+
+        // Assert
+        assertThat(uri1).isNotEqualTo(uri2)
+        // Base paths should be the same
+        assertThat(uri1.buildUpon().clearQuery().build()).isEqualTo(uri2.buildUpon().clearQuery().build())
+    }
+
     @Test
     fun `delete should remove expected avatar path`() = runTest {
         // Arrange

feature/account/profile/api/build.gradle.kts

@@ -9,12 +9,7 @@ kotlin {
     }
     sourceSets {
         commonMain.dependencies {
-            api(projects.feature.account.api)
-            api(projects.feature.account.avatar.api)
-        }
-
-        androidMain.dependencies {
-            // ensure Android target can consume the module if it's platform-specific
+            api(projects.core.architecture.api)
             api(projects.feature.account.api)
             api(projects.feature.account.avatar.api)
         }