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 a8317671982..f86632e83f0 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,17 +15,26 @@ 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 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 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 import retrofit2.Call import retrofit2.Callback @@ -34,10 +42,51 @@ 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 + +/** + * 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. + */ +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: Activity, private var currentUser: User?) { +class PickImage(private val activity: ComponentActivity, private var currentUser: User?) { @Inject lateinit var ncApi: NcApi @@ -112,42 +161,28 @@ 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() + activity.lifecycleScope.launchClosing(body) { + val file = withContext(Dispatchers.IO) { + decodeAvatarToFile(body) { createTempFileForAvatar() } + } + if (file != null) { + selectLocal(startImagePickerForResult, file) + } else { + showAvatarError() + } + } } override fun onFailure(call: Call, t: Throwable) { - // unused atm + Log.e(TAG, "Failed to download the avatar thumbnail", t) + showAvatarError() } }) } - // 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() - 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 { @@ -195,7 +230,6 @@ class PickImage(private val activity: Activity, private var currentUser: User?) 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 00000000000..966f16b2b69 --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/utils/PickImageTest.kt @@ -0,0 +1,88 @@ +/* + * 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 kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +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.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 + 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) + } + + @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() + } +}