Use ShareIntentHandler early to avoid distributing the whole intent (#6274)
* Use `ShareIntentHandler` early to avoid distributing the whole intent This would make the intent be serialized as part of `NavTarget` and could potentially lead to `TransactionTooLargeException`s. We now pass a new `ShareIntentData` class around, containing the minimum amount of data needed. We also have a new `OnSharedData` post-processor to revoke uri access after they've been shared. * Move `UriToShare` next to `ShareIntentData` and add docs
This commit is contained in:
parent
2b04c4bfc0
commit
ae6ea76794
20 changed files with 285 additions and 141 deletions
|
|
@ -0,0 +1,50 @@
|
|||
/*
|
||||
* Copyright (c) 2026 Element Creations Ltd.
|
||||
*
|
||||
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
|
||||
* Please see LICENSE files in the repository root for full details.
|
||||
*/
|
||||
|
||||
package io.element.android.features.share.impl
|
||||
|
||||
import android.content.Context
|
||||
import android.content.Intent
|
||||
import android.net.Uri
|
||||
import android.os.Build
|
||||
import dev.zacsweers.metro.AppScope
|
||||
import dev.zacsweers.metro.ContributesBinding
|
||||
import io.element.android.features.share.api.OnSharedData
|
||||
import io.element.android.features.share.api.ShareIntentData
|
||||
import io.element.android.libraries.di.annotations.ApplicationContext
|
||||
import timber.log.Timber
|
||||
import kotlin.collections.forEach
|
||||
|
||||
@ContributesBinding(AppScope::class)
|
||||
class DefaultOnSharedData(
|
||||
@ApplicationContext private val context: Context,
|
||||
) : OnSharedData {
|
||||
override fun invoke(data: ShareIntentData) {
|
||||
when (data) {
|
||||
is ShareIntentData.PlainText -> {
|
||||
// No-op, there is nothing to do for plain text intents.
|
||||
}
|
||||
is ShareIntentData.Uris -> {
|
||||
revokeUriPermissions(data.uris.map { it.uri })
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private fun revokeUriPermissions(uris: List<Uri>) {
|
||||
uris.forEach { uri ->
|
||||
try {
|
||||
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
|
||||
context.revokeUriPermission(context.packageName, uri, Intent.FLAG_GRANT_READ_URI_PERMISSION)
|
||||
} else {
|
||||
context.revokeUriPermission(uri, Intent.FLAG_GRANT_READ_URI_PERMISSION)
|
||||
}
|
||||
} catch (e: Exception) {
|
||||
Timber.w(e, "Unable to revoke Uri permission")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -26,7 +26,7 @@ class DefaultShareEntryPoint : ShareEntryPoint {
|
|||
return parentNode.createNode<ShareNode>(
|
||||
buildContext = buildContext,
|
||||
plugins = listOf(
|
||||
ShareNode.Inputs(intent = params.intent),
|
||||
ShareNode.Inputs(shareIntentData = params.shareIntentData),
|
||||
callback,
|
||||
)
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1,6 +1,5 @@
|
|||
/*
|
||||
* Copyright (c) 2025 Element Creations Ltd.
|
||||
* Copyright 2024, 2025 New Vector Ltd.
|
||||
* Copyright (c) 2026 Element Creations Ltd.
|
||||
*
|
||||
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
|
||||
* Please see LICENSE files in the repository root for full details.
|
||||
|
|
@ -14,10 +13,12 @@ import android.content.Intent
|
|||
import android.content.pm.PackageManager
|
||||
import android.content.pm.ResolveInfo
|
||||
import android.net.Uri
|
||||
import android.os.Build
|
||||
import androidx.core.content.IntentCompat
|
||||
import dev.zacsweers.metro.AppScope
|
||||
import dev.zacsweers.metro.ContributesBinding
|
||||
import io.element.android.features.share.api.ShareIntentData
|
||||
import io.element.android.features.share.api.ShareIntentHandler
|
||||
import io.element.android.features.share.api.UriToShare
|
||||
import io.element.android.libraries.androidutils.compat.queryIntentActivitiesCompat
|
||||
import io.element.android.libraries.core.mimetype.MimeTypes
|
||||
import io.element.android.libraries.core.mimetype.MimeTypes.isMimeTypeAny
|
||||
|
|
@ -30,37 +31,17 @@ import io.element.android.libraries.core.mimetype.MimeTypes.isMimeTypeVideo
|
|||
import io.element.android.libraries.di.annotations.ApplicationContext
|
||||
import timber.log.Timber
|
||||
|
||||
interface ShareIntentHandler {
|
||||
data class UriToShare(
|
||||
val uri: Uri,
|
||||
val mimeType: String,
|
||||
)
|
||||
|
||||
/**
|
||||
* This methods aims to handle incoming share intents.
|
||||
*
|
||||
* @return true if it can handle the intent data, false otherwise
|
||||
*/
|
||||
suspend fun handleIncomingShareIntent(
|
||||
intent: Intent,
|
||||
onUris: suspend (List<UriToShare>) -> Boolean,
|
||||
onPlainText: suspend (String) -> Boolean,
|
||||
): Boolean
|
||||
}
|
||||
|
||||
@ContributesBinding(AppScope::class)
|
||||
class DefaultShareIntentHandler(
|
||||
@ApplicationContext private val context: Context,
|
||||
) : ShareIntentHandler {
|
||||
override suspend fun handleIncomingShareIntent(
|
||||
override fun handleIncomingShareIntent(
|
||||
intent: Intent,
|
||||
onUris: suspend (List<ShareIntentHandler.UriToShare>) -> Boolean,
|
||||
onPlainText: suspend (String) -> Boolean,
|
||||
): Boolean {
|
||||
val type = intent.resolveType(context) ?: return false
|
||||
): ShareIntentData? {
|
||||
val type = intent.resolveType(context) ?: return null
|
||||
val uris = getIncomingUris(intent, type)
|
||||
return when {
|
||||
uris.isEmpty() && type == MimeTypes.PlainText -> handlePlainText(intent, onPlainText)
|
||||
uris.isEmpty() && type == MimeTypes.PlainText -> handlePlainText(intent)
|
||||
type.isMimeTypeImage() ||
|
||||
type.isMimeTypeVideo() ||
|
||||
type.isMimeTypeAudio() ||
|
||||
|
|
@ -68,20 +49,21 @@ class DefaultShareIntentHandler(
|
|||
type.isMimeTypeFile() ||
|
||||
type.isMimeTypeText() ||
|
||||
type.isMimeTypeAny() -> {
|
||||
val result = onUris(uris)
|
||||
revokeUriPermissions(uris.map { it.uri })
|
||||
result
|
||||
ShareIntentData.Uris(
|
||||
text = intent.getCharSequenceExtra(Intent.EXTRA_TEXT)?.toString()?.takeIf { it.isNotEmpty() },
|
||||
uris = uris,
|
||||
)
|
||||
}
|
||||
else -> false
|
||||
else -> null
|
||||
}
|
||||
}
|
||||
|
||||
private suspend fun handlePlainText(intent: Intent, onPlainText: suspend (String) -> Boolean): Boolean {
|
||||
private fun handlePlainText(intent: Intent): ShareIntentData.PlainText? {
|
||||
val content = intent.getCharSequenceExtra(Intent.EXTRA_TEXT)?.toString()
|
||||
return if (content?.isNotEmpty() == true) {
|
||||
onPlainText(content)
|
||||
ShareIntentData.PlainText(content)
|
||||
} else {
|
||||
false
|
||||
null
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -89,7 +71,7 @@ class DefaultShareIntentHandler(
|
|||
* Use this function to retrieve files which are shared from another application or internally
|
||||
* by using android.intent.action.SEND or android.intent.action.SEND_MULTIPLE actions.
|
||||
*/
|
||||
private fun getIncomingUris(intent: Intent, fallbackMimeType: String): List<ShareIntentHandler.UriToShare> {
|
||||
private fun getIncomingUris(intent: Intent, fallbackMimeType: String): List<UriToShare> {
|
||||
val uriList = mutableListOf<Uri>()
|
||||
if (intent.action == Intent.ACTION_SEND) {
|
||||
IntentCompat.getParcelableExtra(intent, Intent.EXTRA_STREAM, Uri::class.java)
|
||||
|
|
@ -118,24 +100,10 @@ class DefaultShareIntentHandler(
|
|||
// The value in fallbackMimeType can be wrong, especially if several uris were received
|
||||
// in the same intent (i.e. 'image/*'). We need to check the mime type of each uri.
|
||||
val mimeType = context.contentResolver.getType(uri) ?: fallbackMimeType
|
||||
ShareIntentHandler.UriToShare(
|
||||
UriToShare(
|
||||
uri = uri,
|
||||
mimeType = mimeType,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private fun revokeUriPermissions(uris: List<Uri>) {
|
||||
uris.forEach { uri ->
|
||||
try {
|
||||
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
|
||||
context.revokeUriPermission(context.packageName, uri, Intent.FLAG_GRANT_READ_URI_PERMISSION)
|
||||
} else {
|
||||
context.revokeUriPermission(uri, Intent.FLAG_GRANT_READ_URI_PERMISSION)
|
||||
}
|
||||
} catch (e: Exception) {
|
||||
Timber.w(e, "Unable to revoke Uri permission")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -8,7 +8,6 @@
|
|||
|
||||
package io.element.android.features.share.impl
|
||||
|
||||
import android.content.Intent
|
||||
import android.os.Parcelable
|
||||
import androidx.compose.foundation.layout.Box
|
||||
import androidx.compose.runtime.Composable
|
||||
|
|
@ -23,6 +22,7 @@ import dev.zacsweers.metro.Assisted
|
|||
import dev.zacsweers.metro.AssistedInject
|
||||
import io.element.android.annotations.ContributesNode
|
||||
import io.element.android.features.share.api.ShareEntryPoint
|
||||
import io.element.android.features.share.api.ShareIntentData
|
||||
import io.element.android.libraries.architecture.NodeInputs
|
||||
import io.element.android.libraries.architecture.callback
|
||||
import io.element.android.libraries.architecture.inputs
|
||||
|
|
@ -50,10 +50,10 @@ class ShareNode(
|
|||
@Parcelize
|
||||
object NavTarget : Parcelable
|
||||
|
||||
data class Inputs(val intent: Intent) : NodeInputs
|
||||
data class Inputs(val shareIntentData: ShareIntentData) : NodeInputs
|
||||
|
||||
private val inputs = inputs<Inputs>()
|
||||
private val presenter = presenterFactory.create(inputs.intent)
|
||||
private val presenter = presenterFactory.create(inputs.shareIntentData)
|
||||
private val callback: ShareEntryPoint.Callback = callback()
|
||||
|
||||
override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node {
|
||||
|
|
|
|||
|
|
@ -8,13 +8,14 @@
|
|||
|
||||
package io.element.android.features.share.impl
|
||||
|
||||
import android.content.Intent
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.MutableState
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
import dev.zacsweers.metro.Assisted
|
||||
import dev.zacsweers.metro.AssistedFactory
|
||||
import dev.zacsweers.metro.AssistedInject
|
||||
import io.element.android.features.share.api.OnSharedData
|
||||
import io.element.android.features.share.api.ShareIntentData
|
||||
import io.element.android.libraries.architecture.AsyncAction
|
||||
import io.element.android.libraries.architecture.Presenter
|
||||
import io.element.android.libraries.architecture.runCatchingUpdatingState
|
||||
|
|
@ -32,24 +33,24 @@ import kotlin.coroutines.cancellation.CancellationException
|
|||
|
||||
@AssistedInject
|
||||
class SharePresenter(
|
||||
@Assisted private val intent: Intent,
|
||||
@Assisted private val shareIntentData: ShareIntentData,
|
||||
@SessionCoroutineScope
|
||||
private val sessionCoroutineScope: CoroutineScope,
|
||||
private val shareIntentHandler: ShareIntentHandler,
|
||||
private val matrixClient: MatrixClient,
|
||||
private val mediaSenderRoomFactory: MediaSenderRoomFactory,
|
||||
private val activeRoomsHolder: ActiveRoomsHolder,
|
||||
private val mediaOptimizationConfigProvider: MediaOptimizationConfigProvider,
|
||||
private val onSharedData: OnSharedData,
|
||||
) : Presenter<ShareState> {
|
||||
@AssistedFactory
|
||||
fun interface Factory {
|
||||
fun create(intent: Intent): SharePresenter
|
||||
fun create(shareIntentData: ShareIntentData): SharePresenter
|
||||
}
|
||||
|
||||
private val shareActionState: MutableState<AsyncAction<List<RoomId>>> = mutableStateOf(AsyncAction.Uninitialized)
|
||||
|
||||
fun onRoomSelected(roomIds: List<RoomId>) {
|
||||
sessionCoroutineScope.share(intent, roomIds)
|
||||
sessionCoroutineScope.share(shareIntentData, roomIds)
|
||||
}
|
||||
|
||||
@Composable
|
||||
|
|
@ -73,13 +74,24 @@ class SharePresenter(
|
|||
}
|
||||
|
||||
private fun CoroutineScope.share(
|
||||
intent: Intent,
|
||||
shareIntentData: ShareIntentData,
|
||||
roomIds: List<RoomId>,
|
||||
) = launch {
|
||||
suspend {
|
||||
val result = shareIntentHandler.handleIncomingShareIntent(
|
||||
intent,
|
||||
onUris = { filesToShare ->
|
||||
val result = when (shareIntentData) {
|
||||
is ShareIntentData.PlainText -> {
|
||||
roomIds
|
||||
.map { roomId ->
|
||||
getJoinedRoom(roomId)?.liveTimeline?.sendMessage(
|
||||
body = shareIntentData.content,
|
||||
htmlBody = null,
|
||||
intentionalMentions = emptyList(),
|
||||
)?.isSuccess.orFalse()
|
||||
}
|
||||
.all { it }
|
||||
}
|
||||
is ShareIntentData.Uris -> {
|
||||
val filesToShare = shareIntentData.uris
|
||||
if (filesToShare.isEmpty()) {
|
||||
false
|
||||
} else {
|
||||
|
|
@ -90,6 +102,7 @@ class SharePresenter(
|
|||
filesToShare
|
||||
.map { fileToShare ->
|
||||
val result = mediaSender.sendMedia(
|
||||
caption = shareIntentData.text,
|
||||
uri = fileToShare.uri,
|
||||
mimeType = fileToShare.mimeType,
|
||||
mediaOptimizationConfig = mediaOptimizationConfigProvider.get(),
|
||||
|
|
@ -113,19 +126,12 @@ class SharePresenter(
|
|||
}
|
||||
.all { it }
|
||||
}
|
||||
},
|
||||
onPlainText = { text ->
|
||||
roomIds
|
||||
.map { roomId ->
|
||||
getJoinedRoom(roomId)?.liveTimeline?.sendMessage(
|
||||
body = text,
|
||||
htmlBody = null,
|
||||
intentionalMentions = emptyList(),
|
||||
)?.isSuccess.orFalse()
|
||||
}
|
||||
.all { it }
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
// Handle post-processing of shared data
|
||||
onSharedData(shareIntentData)
|
||||
|
||||
if (!result) {
|
||||
error("Failed to handle incoming share intent")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,13 +8,14 @@
|
|||
|
||||
package io.element.android.features.share.impl
|
||||
|
||||
import android.content.Intent
|
||||
import androidx.arch.core.executor.testing.InstantTaskExecutorRule
|
||||
import com.bumble.appyx.core.modality.BuildContext
|
||||
import com.bumble.appyx.testing.junit4.util.MainDispatcherRule
|
||||
import com.google.common.truth.Truth.assertThat
|
||||
import io.element.android.features.share.api.ShareEntryPoint
|
||||
import io.element.android.features.share.api.ShareIntentData
|
||||
import io.element.android.libraries.matrix.api.core.RoomId
|
||||
import io.element.android.libraries.matrix.test.A_MESSAGE
|
||||
import io.element.android.libraries.roomselect.test.FakeRoomSelectEntryPoint
|
||||
import io.element.android.tests.testutils.lambda.lambdaError
|
||||
import io.element.android.tests.testutils.node.TestParentNode
|
||||
|
|
@ -44,7 +45,7 @@ class DefaultShareEntryPointTest {
|
|||
override fun onDone(roomIds: List<RoomId>) = lambdaError()
|
||||
}
|
||||
val params = ShareEntryPoint.Params(
|
||||
intent = Intent(),
|
||||
shareIntentData = ShareIntentData.PlainText(A_MESSAGE),
|
||||
)
|
||||
val result = entryPoint.createNode(
|
||||
parentNode = parentNode,
|
||||
|
|
@ -53,7 +54,7 @@ class DefaultShareEntryPointTest {
|
|||
callback = callback,
|
||||
)
|
||||
assertThat(result).isInstanceOf(ShareNode::class.java)
|
||||
assertThat(result.plugins).contains(ShareNode.Inputs(params.intent))
|
||||
assertThat(result.plugins).contains(ShareNode.Inputs(params.shareIntentData))
|
||||
assertThat(result.plugins).contains(callback)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,27 +0,0 @@
|
|||
/*
|
||||
* Copyright (c) 2025 Element Creations Ltd.
|
||||
* Copyright 2024, 2025 New Vector Ltd.
|
||||
*
|
||||
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
|
||||
* Please see LICENSE files in the repository root for full details.
|
||||
*/
|
||||
|
||||
package io.element.android.features.share.impl
|
||||
|
||||
import android.content.Intent
|
||||
|
||||
class FakeShareIntentHandler(
|
||||
private val onIncomingShareIntent: suspend (
|
||||
Intent,
|
||||
suspend (List<ShareIntentHandler.UriToShare>) -> Boolean,
|
||||
suspend (String) -> Boolean,
|
||||
) -> Boolean = { _, _, _ -> false },
|
||||
) : ShareIntentHandler {
|
||||
override suspend fun handleIncomingShareIntent(
|
||||
intent: Intent,
|
||||
onUris: suspend (List<ShareIntentHandler.UriToShare>) -> Boolean,
|
||||
onPlainText: suspend (String) -> Boolean,
|
||||
): Boolean {
|
||||
return onIncomingShareIntent(intent, onUris, onPlainText)
|
||||
}
|
||||
}
|
||||
|
|
@ -8,12 +8,14 @@
|
|||
|
||||
package io.element.android.features.share.impl
|
||||
|
||||
import android.content.Intent
|
||||
import android.net.Uri
|
||||
import app.cash.molecule.RecompositionMode
|
||||
import app.cash.molecule.moleculeFlow
|
||||
import app.cash.turbine.test
|
||||
import com.google.common.truth.Truth.assertThat
|
||||
import io.element.android.features.share.api.OnSharedData
|
||||
import io.element.android.features.share.api.ShareIntentData
|
||||
import io.element.android.features.share.api.UriToShare
|
||||
import io.element.android.libraries.architecture.AsyncAction
|
||||
import io.element.android.libraries.core.mimetype.MimeTypes
|
||||
import io.element.android.libraries.matrix.api.MatrixClient
|
||||
|
|
@ -72,8 +74,17 @@ class SharePresenterTest {
|
|||
|
||||
@Test
|
||||
fun `present - on room selected ok`() = runTest {
|
||||
val joinedRoom = FakeJoinedRoom(
|
||||
liveTimeline = FakeTimeline().apply {
|
||||
sendMessageLambda = { _, _, _ -> Result.success(Unit) }
|
||||
},
|
||||
)
|
||||
val matrixClient = FakeMatrixClient().apply {
|
||||
givenGetRoomResult(A_ROOM_ID, joinedRoom)
|
||||
}
|
||||
val presenter = createSharePresenter(
|
||||
shareIntentHandler = FakeShareIntentHandler { _, _, _ -> true }
|
||||
matrixClient = matrixClient,
|
||||
shareIntentData = ShareIntentData.PlainText(A_MESSAGE),
|
||||
)
|
||||
moleculeFlow(RecompositionMode.Immediate) {
|
||||
presenter.present()
|
||||
|
|
@ -100,9 +111,7 @@ class SharePresenterTest {
|
|||
}
|
||||
val presenter = createSharePresenter(
|
||||
matrixClient = matrixClient,
|
||||
shareIntentHandler = FakeShareIntentHandler { _, _, onText ->
|
||||
onText(A_MESSAGE)
|
||||
}
|
||||
shareIntentData = ShareIntentData.PlainText(A_MESSAGE),
|
||||
)
|
||||
moleculeFlow(RecompositionMode.Immediate) {
|
||||
presenter.present()
|
||||
|
|
@ -131,16 +140,15 @@ class SharePresenterTest {
|
|||
)
|
||||
val presenter = createSharePresenter(
|
||||
matrixClient = matrixClient,
|
||||
shareIntentHandler = FakeShareIntentHandler { _, onFile, _ ->
|
||||
onFile(
|
||||
listOf(
|
||||
ShareIntentHandler.UriToShare(
|
||||
uri = Uri.parse("content://image.jpg"),
|
||||
mimeType = MimeTypes.Jpeg,
|
||||
)
|
||||
shareIntentData = ShareIntentData.Uris(
|
||||
text = A_MESSAGE,
|
||||
listOf(
|
||||
UriToShare(
|
||||
uri = Uri.parse("content://image.jpg"),
|
||||
mimeType = MimeTypes.Jpeg,
|
||||
)
|
||||
)
|
||||
},
|
||||
),
|
||||
mediaSenderRoomFactory = MediaSenderRoomFactory { mediaSender },
|
||||
)
|
||||
moleculeFlow(RecompositionMode.Immediate) {
|
||||
|
|
@ -159,20 +167,20 @@ class SharePresenterTest {
|
|||
}
|
||||
|
||||
internal fun TestScope.createSharePresenter(
|
||||
intent: Intent = Intent(),
|
||||
shareIntentHandler: ShareIntentHandler = FakeShareIntentHandler(),
|
||||
shareIntentData: ShareIntentData = ShareIntentData.PlainText(A_MESSAGE),
|
||||
matrixClient: MatrixClient = FakeMatrixClient(),
|
||||
activeRoomsHolder: ActiveRoomsHolder = DefaultActiveRoomsHolder(),
|
||||
mediaSenderRoomFactory: MediaSenderRoomFactory = MediaSenderRoomFactory { FakeMediaSender() },
|
||||
mediaOptimizationConfigProvider: MediaOptimizationConfigProvider = FakeMediaOptimizationConfigProvider(),
|
||||
onSharedData: OnSharedData = OnSharedData {},
|
||||
): SharePresenter {
|
||||
return SharePresenter(
|
||||
intent = intent,
|
||||
shareIntentData = shareIntentData,
|
||||
sessionCoroutineScope = this,
|
||||
shareIntentHandler = shareIntentHandler,
|
||||
matrixClient = matrixClient,
|
||||
activeRoomsHolder = activeRoomsHolder,
|
||||
mediaSenderRoomFactory = mediaSenderRoomFactory,
|
||||
mediaOptimizationConfigProvider = mediaOptimizationConfigProvider,
|
||||
onSharedData = onSharedData,
|
||||
)
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue