Skip to content

Commit 03f77d4

Browse files
mahibimahibi
authored
Merge pull request #6863 from ToteMeiSter/fix/provider-main-avatar
fix(avatar): decode and save the remote avatar off the main thread
2 parents 5a2fc92 + ca2a708 commit 03f77d4

2 files changed

Lines changed: 154 additions & 32 deletions

File tree

‎app/src/main/java/com/nextcloud/talk/utils/PickImage.kt‎

Lines changed: 66 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -8,36 +8,85 @@
88

99
package com.nextcloud.talk.utils
1010

11-
import android.app.Activity
1211
import android.content.Intent
1312
import android.graphics.Bitmap
1413
import android.graphics.BitmapFactory
1514
import android.net.Uri
1615
import android.os.Bundle
1716
import android.provider.MediaStore
1817
import android.util.Log
18+
import android.widget.Toast
19+
import androidx.activity.ComponentActivity
1920
import androidx.activity.result.ActivityResultLauncher
21+
import androidx.lifecycle.lifecycleScope
2022
import androidx.core.content.FileProvider
2123
import autodagger.AutoInjector
2224
import com.github.dhaval2404.imagepicker.ImagePicker
2325
import com.github.dhaval2404.imagepicker.constant.ImageProvider
26+
import com.nextcloud.talk.R
2427
import com.nextcloud.talk.api.NcApi
2528
import com.nextcloud.talk.application.NextcloudTalkApplication
2629
import com.nextcloud.talk.data.user.model.User
2730
import com.nextcloud.talk.remotefilebrowser.activities.RemoteFileBrowserActivity
2831
import com.nextcloud.talk.utils.bundle.BundleKeys
2932
import com.nextcloud.talk.utils.permissions.PlatformPermissionUtil
33+
import kotlinx.coroutines.CoroutineScope
34+
import kotlinx.coroutines.Dispatchers
35+
import kotlinx.coroutines.Job
36+
import kotlinx.coroutines.launch
37+
import kotlinx.coroutines.withContext
3038
import okhttp3.ResponseBody
3139
import retrofit2.Call
3240
import retrofit2.Callback
3341
import retrofit2.Response
3442
import java.io.File
3543
import java.io.FileOutputStream
3644
import java.io.IOException
45+
import java.io.InputStream
3746
import javax.inject.Inject
3847

48+
private const val FULL_QUALITY: Int = 100
49+
50+
/**
51+
* Launches [block] and closes [body] when the job completes, including when the scope is already cancelled
52+
* (e.g. the activity was destroyed) and [block] never starts. Closing twice is safe.
53+
*/
54+
internal fun CoroutineScope.launchClosing(body: ResponseBody?, block: suspend CoroutineScope.() -> Unit): Job {
55+
val job = launch(block = block)
56+
job.invokeOnCompletion { body?.close() }
57+
return job
58+
}
59+
60+
/**
61+
* Decodes the avatar thumbnail and writes it as PNG. Blocking, call off the main thread.
62+
* Returns null and logs the cause if the body is missing, does not decode or cannot be written.
63+
*/
64+
internal fun decodeAvatarToFile(
65+
body: ResponseBody?,
66+
decode: (InputStream) -> Bitmap? = { BitmapFactory.decodeStream(it) },
67+
createFile: () -> File
68+
): File? {
69+
val bitmap = body?.use { decode(it.byteStream()) }
70+
if (bitmap == null) {
71+
val cause = if (body == null) "request returned no body" else "could not be decoded"
72+
Log.e("PickImage", "Avatar thumbnail $cause")
73+
return null
74+
}
75+
return try {
76+
val file = createFile()
77+
val written = FileOutputStream(file).use { out ->
78+
bitmap.compress(Bitmap.CompressFormat.PNG, FULL_QUALITY, out)
79+
}
80+
if (!written) throw IOException("Bitmap.compress returned false")
81+
file
82+
} catch (e: IOException) {
83+
Log.e("PickImage", "Error saving the avatar image", e)
84+
null
85+
}
86+
}
87+
3988
@AutoInjector(NextcloudTalkApplication::class)
40-
class PickImage(private val activity: Activity, private var currentUser: User?) {
89+
class PickImage(private val activity: ComponentActivity, private var currentUser: User?) {
4190

4291
@Inject
4392
lateinit var ncApi: NcApi
@@ -112,42 +161,28 @@ class PickImage(private val activity: Activity, private var currentUser: User?)
112161
)
113162
downloadCall.enqueue(object : Callback<ResponseBody> {
114163
override fun onResponse(call: Call<ResponseBody>, response: Response<ResponseBody>) {
115-
saveBitmapAndPassToImagePicker(
116-
startImagePickerForResult,
117-
BitmapFactory.decodeStream(response.body()!!.byteStream())
118-
)
164+
val body = response.body()
165+
activity.lifecycleScope.launchClosing(body) {
166+
val file = withContext(Dispatchers.IO) {
167+
decodeAvatarToFile(body) { createTempFileForAvatar() }
168+
}
169+
if (file != null) {
170+
selectLocal(startImagePickerForResult, file)
171+
} else {
172+
showAvatarError()
173+
}
174+
}
119175
}
120176

121177
override fun onFailure(call: Call<ResponseBody>, t: Throwable) {
122-
// unused atm
178+
Log.e(TAG, "Failed to download the avatar thumbnail", t)
179+
showAvatarError()
123180
}
124181
})
125182
}
126183

127-
// only possible with API26
128-
private fun saveBitmapAndPassToImagePicker(
129-
startImagePickerForResult: ActivityResultLauncher<Intent>,
130-
bitmap: Bitmap
131-
) {
132-
val file: File = saveBitmapToTempFile(bitmap) ?: return
133-
selectLocal(startImagePickerForResult, file)
134-
}
135-
136-
private fun saveBitmapToTempFile(bitmap: Bitmap): File? {
137-
try {
138-
val file = createTempFileForAvatar()
139-
try {
140-
FileOutputStream(file).use { out ->
141-
bitmap.compress(Bitmap.CompressFormat.PNG, FULL_QUALITY, out)
142-
}
143-
return file
144-
} catch (e: IOException) {
145-
Log.e(TAG, "Error compressing bitmap", e)
146-
}
147-
} catch (e: IOException) {
148-
Log.e(TAG, "Error creating temporary avatar image", e)
149-
}
150-
return null
184+
private fun showAvatarError() {
185+
Toast.makeText(activity, R.string.nc_common_error_sorry, Toast.LENGTH_LONG).show()
151186
}
152187

153188
private fun createTempFileForAvatar(): File {
@@ -195,7 +230,6 @@ class PickImage(private val activity: Activity, private var currentUser: User?)
195230
private const val MAX_SIZE: Int = 1024
196231
private const val AVATAR_PATH = "photos/avatar.png"
197232
private const val CAMERA_CAPTURE_PATH = "photos/camera_capture.jpg"
198-
private const val FULL_QUALITY: Int = 100
199233
const val REQUEST_PERMISSION_CAMERA: Int = 1
200234
}
201235
}

‎app/src/test/java/com/nextcloud/talk/utils/PickImageTest.kt‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/*
2+
* Nextcloud Talk - Android Client
3+
*
4+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: GPL-3.0-or-later
6+
*/
7+
8+
package com.nextcloud.talk.utils
9+
10+
import android.graphics.Bitmap
11+
import kotlinx.coroutines.CoroutineScope
12+
import kotlinx.coroutines.Dispatchers
13+
import kotlinx.coroutines.Job
14+
import okhttp3.MediaType.Companion.toMediaType
15+
import okhttp3.ResponseBody
16+
import okhttp3.ResponseBody.Companion.toResponseBody
17+
import org.junit.Assert.assertEquals
18+
import org.junit.Assert.assertNotNull
19+
import org.junit.Assert.assertNull
20+
import org.junit.Assert.assertTrue
21+
import org.junit.Rule
22+
import org.junit.Test
23+
import org.junit.rules.TemporaryFolder
24+
import org.junit.runner.RunWith
25+
import org.mockito.Mockito.mock
26+
import org.mockito.Mockito.verify
27+
import org.robolectric.RobolectricTestRunner
28+
import org.robolectric.annotation.Config
29+
import java.io.ByteArrayOutputStream
30+
import java.io.File
31+
import java.io.IOException
32+
33+
@RunWith(RobolectricTestRunner::class)
34+
@Config(sdk = [33])
35+
class PickImageTest {
36+
37+
@get:Rule
38+
val tmp = TemporaryFolder()
39+
40+
private fun pngBody(): ResponseBody {
41+
val out = ByteArrayOutputStream()
42+
Bitmap.createBitmap(2, 2, Bitmap.Config.ARGB_8888).compress(Bitmap.CompressFormat.PNG, 100, out)
43+
return out.toByteArray().toResponseBody("image/png".toMediaType())
44+
}
45+
46+
@Test
47+
fun nullBodyYieldsNull() {
48+
assertNull(decodeAvatarToFile(null) { tmp.newFile() })
49+
}
50+
51+
@Test
52+
fun undecodableBodyYieldsNull() {
53+
val body = "not an image".toResponseBody("image/png".toMediaType())
54+
assertNull(decodeAvatarToFile(body, decode = { null }) { tmp.newFile() })
55+
}
56+
57+
@Test
58+
fun writeFailureYieldsNull() {
59+
assertNull(decodeAvatarToFile(pngBody()) { throw IOException("disk full") })
60+
}
61+
62+
@Test
63+
fun unwritableTargetYieldsNull() {
64+
val dir: File = tmp.newFolder()
65+
assertNull(decodeAvatarToFile(pngBody()) { dir })
66+
}
67+
68+
@Test
69+
fun validBodyIsSavedAsPng() {
70+
val target = tmp.newFile()
71+
val result = decodeAvatarToFile(pngBody()) { target }
72+
assertNotNull(result)
73+
assertEquals(target, result)
74+
assertTrue(target.length() > 0)
75+
}
76+
77+
@Test
78+
fun bodyIsClosedWhenTheScopeIsAlreadyCancelled() {
79+
val body = mock(ResponseBody::class.java)
80+
var started = false
81+
val scope = CoroutineScope(Job().apply { cancel() } + Dispatchers.Unconfined)
82+
83+
scope.launchClosing(body) { started = true }
84+
85+
assertTrue(!started)
86+
verify(body).close()
87+
}
88+
}

0 commit comments

Comments
 (0)