From 9ca18782bb0c0bd31944f13ba3ee9fe2a022acda Mon Sep 17 00:00:00 2001 From: Krainov Gleb Date: Mon, 12 Oct 2026 12:10:54 +0000 Subject: [PATCH 1/4] fix(avatar): decode and save the remote avatar off the main thread PickImage.handleAvatar decoded the thumbnail stream and wrote the PNG inside the Retrofit callback, which runs on the main thread. Do both in the activity's lifecycleScope on Dispatchers.IO and launch the picker back on the main thread. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb --- .../com/nextcloud/talk/utils/PickImage.kt | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/utils/PickImage.kt b/app/src/main/java/com/nextcloud/talk/utils/PickImage.kt index a831767198..c8e4616535 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/PickImage.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/PickImage.kt @@ -8,7 +8,6 @@ package com.nextcloud.talk.utils -import android.app.Activity import android.content.Intent import android.graphics.Bitmap import android.graphics.BitmapFactory @@ -16,7 +15,9 @@ import android.net.Uri import android.os.Bundle import android.provider.MediaStore import android.util.Log +import androidx.activity.ComponentActivity import androidx.activity.result.ActivityResultLauncher +import androidx.lifecycle.lifecycleScope import androidx.core.content.FileProvider import autodagger.AutoInjector import com.github.dhaval2404.imagepicker.ImagePicker @@ -27,6 +28,9 @@ import com.nextcloud.talk.data.user.model.User import com.nextcloud.talk.remotefilebrowser.activities.RemoteFileBrowserActivity import com.nextcloud.talk.utils.bundle.BundleKeys import com.nextcloud.talk.utils.permissions.PlatformPermissionUtil +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import okhttp3.ResponseBody import retrofit2.Call import retrofit2.Callback @@ -37,7 +41,7 @@ import java.io.IOException import javax.inject.Inject @AutoInjector(NextcloudTalkApplication::class) -class PickImage(private val activity: Activity, private var currentUser: User?) { +class PickImage(private val activity: ComponentActivity, private var currentUser: User?) { @Inject lateinit var ncApi: NcApi @@ -112,10 +116,13 @@ class PickImage(private val activity: Activity, private var currentUser: User?) ) downloadCall.enqueue(object : Callback { override fun onResponse(call: Call, response: Response) { - saveBitmapAndPassToImagePicker( - startImagePickerForResult, - BitmapFactory.decodeStream(response.body()!!.byteStream()) - ) + val body = response.body() ?: return + activity.lifecycleScope.launch { + val file = withContext(Dispatchers.IO) { + BitmapFactory.decodeStream(body.byteStream())?.let { saveBitmapToTempFile(it) } + } + file?.let { selectLocal(startImagePickerForResult, it) } + } } override fun onFailure(call: Call, t: Throwable) { @@ -124,15 +131,6 @@ class PickImage(private val activity: Activity, private var currentUser: User?) }) } - // only possible with API26 - private fun saveBitmapAndPassToImagePicker( - startImagePickerForResult: ActivityResultLauncher, - bitmap: Bitmap - ) { - val file: File = saveBitmapToTempFile(bitmap) ?: return - selectLocal(startImagePickerForResult, file) - } - private fun saveBitmapToTempFile(bitmap: Bitmap): File? { try { val file = createTempFileForAvatar() From 8e4c6f9d54ff1080f6c13c494ecaced7cfff8919 Mon Sep 17 00:00:00 2001 From: Krainov Gleb Date: Tue, 13 Oct 2026 17:56:35 +0000 Subject: [PATCH 2/4] fix(avatar): tell the user when the remote avatar cannot be loaded A response without a body, a thumbnail that does not decode, a failed PNG write and onFailure all ended silently, so picking a file did nothing from the user's side. Log the cause and show R.string.nc_common_error_sorry on the main thread. Close the response body after decoding. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb --- .../com/nextcloud/talk/utils/PickImage.kt | 64 +++++++++++------ .../com/nextcloud/talk/utils/PickImageTest.kt | 69 +++++++++++++++++++ 2 files changed, 113 insertions(+), 20 deletions(-) create mode 100644 app/src/test/java/com/nextcloud/talk/utils/PickImageTest.kt diff --git a/app/src/main/java/com/nextcloud/talk/utils/PickImage.kt b/app/src/main/java/com/nextcloud/talk/utils/PickImage.kt index c8e4616535..68375657e0 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/PickImage.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/PickImage.kt @@ -15,6 +15,7 @@ import android.net.Uri import android.os.Bundle import android.provider.MediaStore import android.util.Log +import android.widget.Toast import androidx.activity.ComponentActivity import androidx.activity.result.ActivityResultLauncher import androidx.lifecycle.lifecycleScope @@ -22,6 +23,7 @@ import androidx.core.content.FileProvider import autodagger.AutoInjector import com.github.dhaval2404.imagepicker.ImagePicker import com.github.dhaval2404.imagepicker.constant.ImageProvider +import com.nextcloud.talk.R import com.nextcloud.talk.api.NcApi import com.nextcloud.talk.application.NextcloudTalkApplication import com.nextcloud.talk.data.user.model.User @@ -38,8 +40,39 @@ import retrofit2.Response import java.io.File import java.io.FileOutputStream import java.io.IOException +import java.io.InputStream import javax.inject.Inject +private const val FULL_QUALITY: Int = 100 + +/** + * Decodes the avatar thumbnail and writes it as PNG. Blocking, call off the main thread. + * Returns null and logs the cause if the body is missing, does not decode or cannot be written. + */ +internal fun decodeAvatarToFile( + body: ResponseBody?, + decode: (InputStream) -> Bitmap? = { BitmapFactory.decodeStream(it) }, + createFile: () -> File +): File? { + val bitmap = body?.use { decode(it.byteStream()) } + if (bitmap == null) { + val cause = if (body == null) "request returned no body" else "could not be decoded" + Log.e("PickImage", "Avatar thumbnail $cause") + return null + } + return try { + val file = createFile() + val written = FileOutputStream(file).use { out -> + bitmap.compress(Bitmap.CompressFormat.PNG, FULL_QUALITY, out) + } + if (!written) throw IOException("Bitmap.compress returned false") + file + } catch (e: IOException) { + Log.e("PickImage", "Error saving the avatar image", e) + null + } +} + @AutoInjector(NextcloudTalkApplication::class) class PickImage(private val activity: ComponentActivity, private var currentUser: User?) { @@ -116,36 +149,28 @@ class PickImage(private val activity: ComponentActivity, private var currentUser ) downloadCall.enqueue(object : Callback { override fun onResponse(call: Call, response: Response) { - val body = response.body() ?: return + val body = response.body() activity.lifecycleScope.launch { val file = withContext(Dispatchers.IO) { - BitmapFactory.decodeStream(body.byteStream())?.let { saveBitmapToTempFile(it) } + decodeAvatarToFile(body) { createTempFileForAvatar() } + } + if (file != null) { + selectLocal(startImagePickerForResult, file) + } else { + showAvatarError() } - file?.let { selectLocal(startImagePickerForResult, it) } } } override fun onFailure(call: Call, t: Throwable) { - // unused atm + Log.e(TAG, "Failed to download the avatar thumbnail", t) + showAvatarError() } }) } - private fun saveBitmapToTempFile(bitmap: Bitmap): File? { - try { - val file = createTempFileForAvatar() - try { - FileOutputStream(file).use { out -> - bitmap.compress(Bitmap.CompressFormat.PNG, FULL_QUALITY, out) - } - return file - } catch (e: IOException) { - Log.e(TAG, "Error compressing bitmap", e) - } - } catch (e: IOException) { - Log.e(TAG, "Error creating temporary avatar image", e) - } - return null + private fun showAvatarError() { + Toast.makeText(activity, R.string.nc_common_error_sorry, Toast.LENGTH_LONG).show() } private fun createTempFileForAvatar(): File { @@ -193,7 +218,6 @@ class PickImage(private val activity: ComponentActivity, private var currentUser private const val MAX_SIZE: Int = 1024 private const val AVATAR_PATH = "photos/avatar.png" private const val CAMERA_CAPTURE_PATH = "photos/camera_capture.jpg" - private const val FULL_QUALITY: Int = 100 const val REQUEST_PERMISSION_CAMERA: Int = 1 } } diff --git a/app/src/test/java/com/nextcloud/talk/utils/PickImageTest.kt b/app/src/test/java/com/nextcloud/talk/utils/PickImageTest.kt new file mode 100644 index 0000000000..cf552fd2a9 --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/utils/PickImageTest.kt @@ -0,0 +1,69 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ + +package com.nextcloud.talk.utils + +import android.graphics.Bitmap +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.ResponseBody +import okhttp3.ResponseBody.Companion.toResponseBody +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import java.io.ByteArrayOutputStream +import java.io.File +import java.io.IOException + +@RunWith(RobolectricTestRunner::class) +class PickImageTest { + + @get:Rule + val tmp = TemporaryFolder() + + private fun pngBody(): ResponseBody { + val out = ByteArrayOutputStream() + Bitmap.createBitmap(2, 2, Bitmap.Config.ARGB_8888).compress(Bitmap.CompressFormat.PNG, 100, out) + return out.toByteArray().toResponseBody("image/png".toMediaType()) + } + + @Test + fun nullBodyYieldsNull() { + assertNull(decodeAvatarToFile(null) { tmp.newFile() }) + } + + @Test + fun undecodableBodyYieldsNull() { + val body = "not an image".toResponseBody("image/png".toMediaType()) + assertNull(decodeAvatarToFile(body, decode = { null }) { tmp.newFile() }) + } + + @Test + fun writeFailureYieldsNull() { + assertNull(decodeAvatarToFile(pngBody()) { throw IOException("disk full") }) + } + + @Test + fun unwritableTargetYieldsNull() { + val dir: File = tmp.newFolder() + assertNull(decodeAvatarToFile(pngBody()) { dir }) + } + + @Test + fun validBodyIsSavedAsPng() { + val target = tmp.newFile() + val result = decodeAvatarToFile(pngBody()) { target } + assertNotNull(result) + assertEquals(target, result) + assertTrue(target.length() > 0) + } +} From a0d9a7820d6518a2c4ce243011d3a68b05e60ab3 Mon Sep 17 00:00:00 2001 From: Krainov Gleb Date: Tue, 13 Oct 2026 18:14:13 +0000 Subject: [PATCH 3/4] fix(avatar): close the thumbnail body when the activity is already destroyed If the activity is destroyed between onResponse and the coroutine start, lifecycleScope.launch never runs the block, so the body returned by Retrofit stayed open. Close it from the job's completion handler; closing twice is safe. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb --- .../java/com/nextcloud/talk/utils/PickImage.kt | 14 +++++++++++++- .../com/nextcloud/talk/utils/PickImageTest.kt | 17 +++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/nextcloud/talk/utils/PickImage.kt b/app/src/main/java/com/nextcloud/talk/utils/PickImage.kt index 68375657e0..f86632e83f 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/PickImage.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/PickImage.kt @@ -30,7 +30,9 @@ import com.nextcloud.talk.data.user.model.User import com.nextcloud.talk.remotefilebrowser.activities.RemoteFileBrowserActivity import com.nextcloud.talk.utils.bundle.BundleKeys import com.nextcloud.talk.utils.permissions.PlatformPermissionUtil +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import okhttp3.ResponseBody @@ -45,6 +47,16 @@ import javax.inject.Inject private const val FULL_QUALITY: Int = 100 +/** + * Launches [block] and closes [body] when the job completes, including when the scope is already cancelled + * (e.g. the activity was destroyed) and [block] never starts. Closing twice is safe. + */ +internal fun CoroutineScope.launchClosing(body: ResponseBody?, block: suspend CoroutineScope.() -> Unit): Job { + val job = launch(block = block) + job.invokeOnCompletion { body?.close() } + return job +} + /** * Decodes the avatar thumbnail and writes it as PNG. Blocking, call off the main thread. * Returns null and logs the cause if the body is missing, does not decode or cannot be written. @@ -150,7 +162,7 @@ class PickImage(private val activity: ComponentActivity, private var currentUser downloadCall.enqueue(object : Callback { override fun onResponse(call: Call, response: Response) { val body = response.body() - activity.lifecycleScope.launch { + activity.lifecycleScope.launchClosing(body) { val file = withContext(Dispatchers.IO) { decodeAvatarToFile(body) { createTempFileForAvatar() } } diff --git a/app/src/test/java/com/nextcloud/talk/utils/PickImageTest.kt b/app/src/test/java/com/nextcloud/talk/utils/PickImageTest.kt index cf552fd2a9..b79dbf64f0 100644 --- a/app/src/test/java/com/nextcloud/talk/utils/PickImageTest.kt +++ b/app/src/test/java/com/nextcloud/talk/utils/PickImageTest.kt @@ -8,6 +8,9 @@ package com.nextcloud.talk.utils import android.graphics.Bitmap +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job import okhttp3.MediaType.Companion.toMediaType import okhttp3.ResponseBody import okhttp3.ResponseBody.Companion.toResponseBody @@ -19,6 +22,8 @@ import org.junit.Rule import org.junit.Test import org.junit.rules.TemporaryFolder import org.junit.runner.RunWith +import org.mockito.Mockito.mock +import org.mockito.Mockito.verify import org.robolectric.RobolectricTestRunner import java.io.ByteArrayOutputStream import java.io.File @@ -66,4 +71,16 @@ class PickImageTest { assertEquals(target, result) assertTrue(target.length() > 0) } + + @Test + fun bodyIsClosedWhenTheScopeIsAlreadyCancelled() { + val body = mock(ResponseBody::class.java) + var started = false + val scope = CoroutineScope(Job().apply { cancel() } + Dispatchers.Unconfined) + + scope.launchClosing(body) { started = true } + + assertTrue(!started) + verify(body).close() + } } From ca2a708fdedd7ba49a462f220628baafb8798b43 Mon Sep 17 00:00:00 2001 From: Krainov Gleb Date: Thu, 15 Oct 2026 06:19:21 +0000 Subject: [PATCH 4/4] test(avatar): run PickImageTest on API 33 like the other Robolectric tests Without @Config the runner picks the target SDK (36) and has to download a second android-all jar. On CI that download failed with an SSLException, so the class failed with initializationError before any test ran. Signed-off-by: Krainov Gleb Assisted-by: Claude-Code:claude-opus-5-5 --- app/src/test/java/com/nextcloud/talk/utils/PickImageTest.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/src/test/java/com/nextcloud/talk/utils/PickImageTest.kt b/app/src/test/java/com/nextcloud/talk/utils/PickImageTest.kt index b79dbf64f0..966f16b2b6 100644 --- a/app/src/test/java/com/nextcloud/talk/utils/PickImageTest.kt +++ b/app/src/test/java/com/nextcloud/talk/utils/PickImageTest.kt @@ -25,11 +25,13 @@ import org.junit.runner.RunWith import org.mockito.Mockito.mock import org.mockito.Mockito.verify import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config import java.io.ByteArrayOutputStream import java.io.File import java.io.IOException @RunWith(RobolectricTestRunner::class) +@Config(sdk = [33]) class PickImageTest { @get:Rule