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:
committed by
GitHub
parent
7c97ec1155
commit
9c8757e38b
@@ -57,6 +57,7 @@ dependencies {
|
||||
|
||||
testCommonDependencies(libs)
|
||||
testImplementation(projects.features.login.test)
|
||||
testImplementation(projects.features.share.test)
|
||||
testImplementation(projects.libraries.matrix.test)
|
||||
testImplementation(projects.libraries.oidc.test)
|
||||
testImplementation(projects.libraries.preferences.test)
|
||||
|
||||
@@ -8,7 +8,6 @@
|
||||
|
||||
package io.element.android.appnav
|
||||
|
||||
import android.content.Intent
|
||||
import android.os.Parcelable
|
||||
import androidx.compose.foundation.layout.Box
|
||||
import androidx.compose.runtime.Composable
|
||||
@@ -63,6 +62,7 @@ import io.element.android.features.roomdirectory.api.RoomDescription
|
||||
import io.element.android.features.roomdirectory.api.RoomDirectoryEntryPoint
|
||||
import io.element.android.features.securebackup.api.SecureBackupEntryPoint
|
||||
import io.element.android.features.share.api.ShareEntryPoint
|
||||
import io.element.android.features.share.api.ShareIntentData
|
||||
import io.element.android.features.startchat.api.StartChatEntryPoint
|
||||
import io.element.android.features.userprofile.api.UserProfileEntryPoint
|
||||
import io.element.android.features.verifysession.api.IncomingVerificationEntryPoint
|
||||
@@ -307,7 +307,7 @@ class LoggedInFlowNode(
|
||||
data object RoomDirectory : NavTarget
|
||||
|
||||
@Parcelize
|
||||
data class IncomingShare(val intent: Intent) : NavTarget
|
||||
data class IncomingShare(val shareIntentData: ShareIntentData) : NavTarget
|
||||
|
||||
@Parcelize
|
||||
data class IncomingVerificationRequest(val data: VerificationRequest.Incoming) : NavTarget
|
||||
@@ -570,7 +570,7 @@ class LoggedInFlowNode(
|
||||
shareEntryPoint.createNode(
|
||||
parentNode = this,
|
||||
buildContext = buildContext,
|
||||
params = ShareEntryPoint.Params(intent = navTarget.intent),
|
||||
params = ShareEntryPoint.Params(shareIntentData = navTarget.shareIntentData),
|
||||
callback = object : ShareEntryPoint.Callback {
|
||||
override fun onDone(roomIds: List<RoomId>) {
|
||||
// Remove the incoming share screen
|
||||
@@ -649,13 +649,13 @@ class LoggedInFlowNode(
|
||||
}
|
||||
}
|
||||
|
||||
internal suspend fun attachIncomingShare(intent: Intent) {
|
||||
internal suspend fun attachIncomingShare(shareIntentData: ShareIntentData) {
|
||||
waitForNavTargetAttached { navTarget ->
|
||||
navTarget is NavTarget.Home
|
||||
}
|
||||
attachChild<Node> {
|
||||
backstack.push(
|
||||
NavTarget.IncomingShare(intent)
|
||||
NavTarget.IncomingShare(shareIntentData)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -44,6 +44,7 @@ import io.element.android.features.announcement.api.AnnouncementService
|
||||
import io.element.android.features.login.api.LoginParams
|
||||
import io.element.android.features.login.api.accesscontrol.AccountProviderAccessControl
|
||||
import io.element.android.features.rageshake.api.bugreport.BugReportEntryPoint
|
||||
import io.element.android.features.share.api.ShareIntentData
|
||||
import io.element.android.features.signedout.api.SignedOutEntryPoint
|
||||
import io.element.android.libraries.accountselect.api.AccountSelectEntryPoint
|
||||
import io.element.android.libraries.architecture.BackstackView
|
||||
@@ -265,7 +266,7 @@ class RootFlowNode(
|
||||
|
||||
@Parcelize data class AccountSelect(
|
||||
val currentSessionId: SessionId,
|
||||
val intent: Intent?,
|
||||
val shareIntentData: ShareIntentData?,
|
||||
val permalinkData: PermalinkData?,
|
||||
) : NavTarget
|
||||
|
||||
@@ -357,8 +358,8 @@ class RootFlowNode(
|
||||
backstack.pop()
|
||||
}
|
||||
attachSession(sessionId).apply {
|
||||
if (navTarget.intent != null) {
|
||||
attachIncomingShare(navTarget.intent)
|
||||
if (navTarget.shareIntentData != null) {
|
||||
attachIncomingShare(navTarget.shareIntentData)
|
||||
} else if (navTarget.permalinkData != null) {
|
||||
attachPermalinkData(navTarget.permalinkData)
|
||||
}
|
||||
@@ -392,7 +393,7 @@ class RootFlowNode(
|
||||
is ResolvedIntent.Login -> onLoginLink(resolvedIntent.params)
|
||||
is ResolvedIntent.Oidc -> onOidcAction(resolvedIntent.oidcAction)
|
||||
is ResolvedIntent.Permalink -> navigateTo(resolvedIntent.permalinkData)
|
||||
is ResolvedIntent.IncomingShare -> onIncomingShare(resolvedIntent.intent)
|
||||
is ResolvedIntent.IncomingShare -> onIncomingShare(resolvedIntent.shareIntentData)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -423,7 +424,7 @@ class RootFlowNode(
|
||||
}
|
||||
}
|
||||
|
||||
private suspend fun onIncomingShare(intent: Intent) {
|
||||
private suspend fun onIncomingShare(shareIntentData: ShareIntentData) {
|
||||
// Is there a session already?
|
||||
val latestSessionId = sessionStore.getLatestSessionId()
|
||||
if (latestSessionId == null) {
|
||||
@@ -437,13 +438,13 @@ class RootFlowNode(
|
||||
backstack.push(
|
||||
NavTarget.AccountSelect(
|
||||
currentSessionId = latestSessionId,
|
||||
intent = intent,
|
||||
shareIntentData = shareIntentData,
|
||||
permalinkData = null,
|
||||
)
|
||||
)
|
||||
} else {
|
||||
// Only one account, directly attach the incoming share node.
|
||||
loggedInFlowNode.attachIncomingShare(intent)
|
||||
loggedInFlowNode.attachIncomingShare(shareIntentData)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -467,7 +468,7 @@ class RootFlowNode(
|
||||
backstack.push(
|
||||
NavTarget.AccountSelect(
|
||||
currentSessionId = latestSessionId,
|
||||
intent = null,
|
||||
shareIntentData = null,
|
||||
permalinkData = permalinkData,
|
||||
)
|
||||
)
|
||||
|
||||
@@ -12,6 +12,8 @@ import android.content.Intent
|
||||
import dev.zacsweers.metro.Inject
|
||||
import io.element.android.features.login.api.LoginIntentResolver
|
||||
import io.element.android.features.login.api.LoginParams
|
||||
import io.element.android.features.share.api.ShareIntentData
|
||||
import io.element.android.features.share.api.ShareIntentHandler
|
||||
import io.element.android.libraries.deeplink.api.DeeplinkData
|
||||
import io.element.android.libraries.deeplink.api.DeeplinkParser
|
||||
import io.element.android.libraries.matrix.api.permalink.PermalinkData
|
||||
@@ -25,7 +27,7 @@ sealed interface ResolvedIntent {
|
||||
data class Oidc(val oidcAction: OidcAction) : ResolvedIntent
|
||||
data class Permalink(val permalinkData: PermalinkData) : ResolvedIntent
|
||||
data class Login(val params: LoginParams) : ResolvedIntent
|
||||
data class IncomingShare(val intent: Intent) : ResolvedIntent
|
||||
data class IncomingShare(val shareIntentData: ShareIntentData) : ResolvedIntent
|
||||
}
|
||||
|
||||
@Inject
|
||||
@@ -34,6 +36,7 @@ class IntentResolver(
|
||||
private val loginIntentResolver: LoginIntentResolver,
|
||||
private val oidcIntentResolver: OidcIntentResolver,
|
||||
private val permalinkParser: PermalinkParser,
|
||||
private val shareIntentHandler: ShareIntentHandler,
|
||||
) {
|
||||
fun resolve(intent: Intent): ResolvedIntent? {
|
||||
if (intent.canBeIgnored()) return null
|
||||
@@ -62,7 +65,8 @@ class IntentResolver(
|
||||
if (permalinkData != null) return ResolvedIntent.Permalink(permalinkData)
|
||||
|
||||
if (intent.action == Intent.ACTION_SEND || intent.action == Intent.ACTION_SEND_MULTIPLE) {
|
||||
return ResolvedIntent.IncomingShare(intent)
|
||||
val data = shareIntentHandler.handleIncomingShareIntent(intent) ?: return null
|
||||
return ResolvedIntent.IncomingShare(data)
|
||||
}
|
||||
|
||||
// Unknown intent
|
||||
|
||||
@@ -15,6 +15,9 @@ import androidx.core.net.toUri
|
||||
import com.google.common.truth.Truth.assertThat
|
||||
import io.element.android.features.login.api.LoginParams
|
||||
import io.element.android.features.login.test.FakeLoginIntentResolver
|
||||
import io.element.android.features.share.api.ShareIntentData
|
||||
import io.element.android.features.share.api.UriToShare
|
||||
import io.element.android.features.share.test.FakeShareIntentHandler
|
||||
import io.element.android.libraries.deeplink.api.DeeplinkData
|
||||
import io.element.android.libraries.matrix.api.core.UserId
|
||||
import io.element.android.libraries.matrix.api.permalink.PermalinkData
|
||||
@@ -239,26 +242,34 @@ class IntentResolverTest {
|
||||
|
||||
@Test
|
||||
fun `test incoming share simple`() {
|
||||
val shareIntentData = ShareIntentData.PlainText("Hello")
|
||||
val sut = createIntentResolver(
|
||||
oidcIntentResolverResult = { null },
|
||||
onIncomingShareIntent = { shareIntentData },
|
||||
)
|
||||
val intent = Intent(RuntimeEnvironment.getApplication(), Activity::class.java).apply {
|
||||
action = Intent.ACTION_SEND
|
||||
putExtra(Intent.EXTRA_TEXT, "Hello")
|
||||
}
|
||||
val result = sut.resolve(intent)
|
||||
assertThat(result).isEqualTo(ResolvedIntent.IncomingShare(intent = intent))
|
||||
assertThat(result).isEqualTo(ResolvedIntent.IncomingShare(shareIntentData))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `test incoming share multiple`() {
|
||||
val fileUri = "content://com.example.app/file1.jpg".toUri()
|
||||
val shareIntentData = ShareIntentData.Uris(text = "Hello", uris = listOf(UriToShare(fileUri, "image/jpg")))
|
||||
val sut = createIntentResolver(
|
||||
oidcIntentResolverResult = { null },
|
||||
onIncomingShareIntent = { shareIntentData },
|
||||
)
|
||||
val intent = Intent(RuntimeEnvironment.getApplication(), Activity::class.java).apply {
|
||||
action = Intent.ACTION_SEND_MULTIPLE
|
||||
putExtra(Intent.EXTRA_TEXT, "Hello")
|
||||
data = fileUri
|
||||
}
|
||||
val result = sut.resolve(intent)
|
||||
assertThat(result).isEqualTo(ResolvedIntent.IncomingShare(intent = intent))
|
||||
assertThat(result).isEqualTo(ResolvedIntent.IncomingShare(shareIntentData))
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -296,6 +307,7 @@ class IntentResolverTest {
|
||||
permalinkParserResult: (String) -> PermalinkData = { lambdaError() },
|
||||
loginIntentResolverResult: (String) -> LoginParams? = { lambdaError() },
|
||||
oidcIntentResolverResult: (Intent) -> OidcAction? = { lambdaError() },
|
||||
onIncomingShareIntent: (Intent) -> ShareIntentData? = { null },
|
||||
): IntentResolver {
|
||||
return IntentResolver(
|
||||
deeplinkParser = { deeplinkParserResult },
|
||||
@@ -308,6 +320,9 @@ class IntentResolverTest {
|
||||
permalinkParser = FakePermalinkParser(
|
||||
result = permalinkParserResult
|
||||
),
|
||||
shareIntentHandler = FakeShareIntentHandler(
|
||||
onIncomingShareIntent = onIncomingShareIntent,
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user