refactor: use OpenDocument picker and support PNG mime types

Author: mcbaumwolle

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

@@ -3,6 +3,8 @@ package net.thunderbird.feature.account.avatar.data.datasource
 import com.eygraber.uri.Uri
 import net.thunderbird.core.file.DirectoryProvider
 import net.thunderbird.core.file.FileManager
+import net.thunderbird.core.file.MimeType
+import net.thunderbird.core.file.MimeTypeResolver
 import net.thunderbird.feature.account.AccountId
 import net.thunderbird.feature.account.avatar.data.AvatarDataContract.DataSource.LocalAvatarImage
 
@@ -14,25 +16,27 @@ import net.thunderbird.feature.account.avatar.data.AvatarDataContract.DataSource
  *
  * @param fileManager The [FileManager] for file operations.
  * @param directoryProvider The [DirectoryProvider] to get app directories.
+ * @param mimeTypeResolver The [MimeTypeResolver] to detect image formats.
  */
 internal class LocalAvatarImageDataSource(
     private val fileManager: FileManager,
     private val directoryProvider: DirectoryProvider,
+    private val mimeTypeResolver: MimeTypeResolver,
 ) : LocalAvatarImage {
 
     override suspend fun update(id: AccountId, imageUri: Uri): Uri {
-        // Detect desired extension from input (simple heuristic)
-        val isPng = imageUri.toString().endsWith(".png", ignoreCase = true)
+        // Using the resolver to detect the actual (MIME) type
+        val mimeType = mimeTypeResolver.getMimeType(imageUri)
+        val isPng = mimeType == MimeType.PNG
+
+        // Set the correct extension (.png or .jpg)
         val targetExtension = if (isPng) EXTENSION_PNG else EXTENSION_JPG
 
-        // 1. Clean up any existing avatars (jpg or png) for this account
-        //    to avoid having "123.jpg" and "123.png" existing simultaneously.
+        // Cleaning up old files
         delete(id)
 
-        // 2. Generate the new target URI
+        // 4. Generating the new path and copy
         val avatarImageUri = getAvatarImageUri(id, targetExtension)
-
-        // 3. Copy the file
         fileManager.copy(imageUri, avatarImageUri)
 
         return avatarImageUri

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

@@ -11,6 +11,7 @@ val featureAccountAvatarModule = module {
         LocalAvatarImageDataSource(
             fileManager = get(),
             directoryProvider = get(),
+            mimeTypeResolver = get(),
         )
     }
 

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

@@ -11,7 +11,10 @@ import kotlinx.coroutines.test.runTest
 import net.thunderbird.core.file.DirectoryProvider
 import net.thunderbird.core.file.FileManager
 import net.thunderbird.core.file.FileOperationError
+import net.thunderbird.core.file.MimeType
+import net.thunderbird.core.file.MimeTypeResolver
 import net.thunderbird.core.outcome.Outcome
+import net.thunderbird.feature.account.AccountId
 import net.thunderbird.feature.account.AccountIdFactory
 import net.thunderbird.feature.account.avatar.data.AvatarDataContract
 import org.junit.Rule
@@ -25,24 +28,29 @@ class LocalAvatarImageDataSourceTest {
 
     private lateinit var directoryProvider: DirectoryProvider
     private lateinit var fileManager: SpyFileManager
+    private lateinit var mimeTypeResolver: FakeMimeTypeResolver
     private lateinit var testSubject: LocalAvatarImageDataSource
 
     @BeforeTest
     fun setUp() {
         val appDir = folder.newFolder("app")
         directoryProvider = FakeDirectoryProvider(appDir.absolutePath.toKmpUri())
         fileManager = SpyFileManager()
-        testSubject = LocalAvatarImageDataSource(fileManager, directoryProvider)
+        mimeTypeResolver = FakeMimeTypeResolver()
+        testSubject = LocalAvatarImageDataSource(fileManager, directoryProvider, mimeTypeResolver)
     }
 
     @Test
     fun `update with JPG should copy image to JPG path and clean up old files`() = runTest {
         // Arrange
         val accountId = AccountIdFactory.create()
         val source = "file:///external/picked/image.jpg".toKmpUri()
-        val expectedDir = getAvatarDir()
+        val expectedDir = getAvatarDir(accountId)
         val expectedDest = expectedDir.buildUpon().appendPath("$accountId.jpg").build()
 
+        // Tell the fake that this source is a JPEG
+        mimeTypeResolver.mimeTypeToReturn = MimeType.JPEG
+
         // Act
         val returned = testSubject.update(accountId, source)
 
@@ -51,7 +59,7 @@ class LocalAvatarImageDataSourceTest {
         assertThat(fileManager.lastCopySource).isEqualTo(source)
         assertThat(fileManager.lastCopyDestination).isEqualTo(expectedDest)
 
-        // Verify
+        // Verify cleanup
         val expectedPngDel = expectedDir.buildUpon().appendPath("$accountId.png").build()
         val expectedJpgDel = expectedDir.buildUpon().appendPath("$accountId.jpg").build()
         assertThat(fileManager.deletedPaths).contains(expectedPngDel)
@@ -63,9 +71,12 @@ class LocalAvatarImageDataSourceTest {
         // Arrange
         val accountId = AccountIdFactory.create()
         val source = "file:///external/picked/photo.png".toKmpUri()
-        val expectedDir = getAvatarDir()
+        val expectedDir = getAvatarDir(accountId)
         val expectedDest = expectedDir.buildUpon().appendPath("$accountId.png").build()
 
+        // Tell the fake that this source is a PNG
+        mimeTypeResolver.mimeTypeToReturn = MimeType.PNG
+
         // Act
         val returned = testSubject.update(accountId, source)
 
@@ -79,7 +90,7 @@ class LocalAvatarImageDataSourceTest {
     fun `delete should remove both JPG and PNG paths`() = runTest {
         // Arrange
         val accountId = AccountIdFactory.create()
-        val expectedDir = getAvatarDir()
+        val expectedDir = getAvatarDir(accountId)
         val jpgPath = expectedDir.buildUpon().appendPath("$accountId.jpg").build()
         val pngPath = expectedDir.buildUpon().appendPath("$accountId.png").build()
 
@@ -91,13 +102,22 @@ class LocalAvatarImageDataSourceTest {
         assertThat(fileManager.deletedPaths).contains(pngPath)
     }
 
-    private suspend fun getAvatarDir(): Uri {
+    private suspend fun getAvatarDir(accountId: AccountId): Uri {
         return directoryProvider.getFilesDir().buildUpon()
             .appendPath(AvatarDataContract.DataSource.LocalAvatarImage.DIRECTORY_NAME)
             .build()
     }
 
-    // Fixed SpyFileManager to match interface return types
+    // --- Helpers ---
+
+    class FakeMimeTypeResolver : MimeTypeResolver {
+        var mimeTypeToReturn: MimeType? = null
+
+        override fun getMimeType(uri: Uri): MimeType? {
+            return mimeTypeToReturn
+        }
+    }
+
     class SpyFileManager : FileManager {
         var lastCopySource: Uri? = null
         var lastCopyDestination: Uri? = null
@@ -117,5 +137,8 @@ class LocalAvatarImageDataSourceTest {
         override suspend fun createDirectories(uri: Uri): Outcome<Unit, FileOperationError> {
             return Outcome.Success(Unit)
         }
+
+        // Add other methods if FileManager interface requires them (e.g., exists, listFiles)
+        // override suspend fun exists(uri: Uri): Boolean = true
     }
 }

feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/domain/usecase/UpdateAvatarImage.kt

@@ -22,10 +22,11 @@ internal class UpdateAvatarImage(
     ): Outcome<Avatar.Image, AccountSettingError> {
         val mimeType = mimeTypeResolver.getMimeType(imageUri)
 
-        if (mimeType == null || mimeType != MimeType.JPEG) {
+        // Check for both JPEG and PNG
+        if (mimeType == null || (mimeType != MimeType.JPEG && mimeType != MimeType.PNG)) {
             return Outcome.Failure(
                 AccountSettingError.UnsupportedFormat(
-                    message = "Only JPEG images are supported. Found: ${mimeType ?: "unknown"}",
+                    message = "Only JPEG and PNG images are supported. Found: ${mimeType ?: "unknown"}",
                 ),
             )
         }

feature/account/settings/impl/src/main/kotlin/net/thunderbird/feature/account/settings/impl/ui/general/GeneralSettingsScreen.kt

@@ -25,7 +25,7 @@ internal fun GeneralSettingsScreen(
     provider: SettingViewProvider = koinInject(),
     builder: SettingsBuilder = koinInject(),
 ) {
-    val imagePicker = rememberLauncherForActivityResult(ActivityResultContracts.GetContent()) { uri ->
+    val imagePicker = rememberLauncherForActivityResult(ActivityResultContracts.OpenDocument()) { uri ->
         if (uri != null) {
             viewModel.event(Event.OnAvatarImagePicked(uri.toKmpUri()))
         }
@@ -35,7 +35,7 @@ internal fun GeneralSettingsScreen(
         when (effect) {
             is Effect.NavigateBack -> onBack()
             is Effect.OpenAvatarImagePicker -> {
-                imagePicker.launch("image/*")
+                imagePicker.launch(arrayOf("image/jpeg", "image/png"))
             }
         }
     }