Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update mobile connect flow and UI #1008

Merged
merged 13 commits into from
Jun 27, 2024
Merged
9 changes: 0 additions & 9 deletions app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,6 @@

<category android:name="android.intent.category.LAUNCHER" />
</intent-filter>
<intent-filter android:autoVerify="true">
<action android:name="android.intent.action.VIEW" />

<category android:name="android.intent.category.DEFAULT" />
<category android:name="android.intent.category.BROWSABLE" />

<data android:scheme="https" />
<data android:host="dr6vsuukf8610.cloudfront.net" />
</intent-filter>
<intent-filter>
<action android:name="android.intent.action.VIEW" />

Expand Down
10 changes: 8 additions & 2 deletions app/src/main/java/com/babylon/wallet/android/MainActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ class MainActivity : FragmentActivity() {
super.onCreate(savedInstanceState)
cloudBackupSyncExecutor.startPeriodicChecks(lifecycleOwner = this)

intent.data?.let { viewModel.handleDeepLink(it) }
intent.data?.let {
intent.replaceExtras(Bundle())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice that you added this here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually can someone explain me what does this line do? Because I see it in the line 103, too.
Does it "clear" the intent with an empty bundle?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it removes data from an intent, so deep link is not delivered 2nd time if you move app to background and open from recent

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the app is moved to background and re-opened from recent neither onCreate nor onNewIntent should get called in normal circumstances, unless the app task has been closed by the OS? Is that the case or there are simple repro steps for the issue that this line fixes? I'm really curious about what is happening.

viewModel.handleDeepLink(it)
}
setContent {
RadixWalletTheme {
val isVisible by balanceVisibilityObserver.isBalanceVisible.collectAsState(initial = true)
Expand Down Expand Up @@ -100,7 +103,10 @@ class MainActivity : FragmentActivity() {

override fun onNewIntent(intent: Intent) {
super.onNewIntent(intent)
intent.data?.let { viewModel.handleDeepLink(it) }
intent.data?.let {
this.intent.replaceExtras(Bundle())
viewModel.handleDeepLink(it)
}
}

private fun setSplashExitAnimation(splashScreen: SplashScreen) {
Expand Down
22 changes: 11 additions & 11 deletions app/src/main/java/com/babylon/wallet/android/WalletApp.kt
Original file line number Diff line number Diff line change
Expand Up @@ -68,26 +68,26 @@ fun WalletApp(
mainViewModel.oneOffEvent.collect { event ->
when (event) {
is MainEvent.IncomingRequestEvent -> {
if (event.request.needVerification) {
navController.mobileConnect(event.request.interactionId)
return@collect
}
when (val incomingRequest = event.request) {
is IncomingMessage.IncomingRequest.TransactionRequest -> {
navController.transactionReview(
requestId = incomingRequest.interactionId.toString()
requestId = incomingRequest.interactionId
)
}

is IncomingMessage.IncomingRequest.AuthorizedRequest -> {
navController.dAppLoginAuthorized(incomingRequest.interactionId.toString())
navController.dAppLoginAuthorized(incomingRequest.interactionId)
}

is IncomingMessage.IncomingRequest.UnauthorizedRequest -> {
navController.dAppLoginUnauthorized(incomingRequest.interactionId.toString())
navController.dAppLoginUnauthorized(incomingRequest.interactionId)
}
}
}

is MainEvent.MobileConnectLink -> {
navController.mobileConnect(event.request)
}
}
}
}
Expand Down Expand Up @@ -177,13 +177,13 @@ fun WalletApp(
dismissText = null
)
}
if (!state.isProfileInitialized) {
if (state.showMobileConnectWarning) {
BasicPromptAlertDialog(
finish = {
onCloseApp()
mainViewModel.onMobileConnectWarningShown()
},
titleText = "No profile found",
messageText = "You need to create a profile to respond to dApp requests",
titleText = stringResource(id = R.string.mobileConnect_noProfileDialog_title),
messageText = stringResource(id = R.string.mobileConnect_noProfileDialog_subtitle),
confirmText = stringResource(id = R.string.common_ok),
dismissText = null
)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.babylon.wallet.android.data.dapp

import com.babylon.wallet.android.domain.model.IncomingMessage.IncomingRequest
import com.babylon.wallet.android.utils.AppEvent
import com.babylon.wallet.android.utils.AppEventBus
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.asSharedFlow
Expand All @@ -17,24 +19,31 @@ interface IncomingRequestRepository {

suspend fun add(incomingRequest: IncomingRequest)

suspend fun addPriorityRequest(incomingRequest: IncomingRequest)

suspend fun requestHandled(requestId: String)

suspend fun pauseIncomingRequests()

suspend fun resumeIncomingRequests()

fun getUnauthorizedRequest(requestId: String): IncomingRequest.UnauthorizedRequest?

fun getTransactionWriteRequest(requestId: String): IncomingRequest.TransactionRequest?

fun getAuthorizedRequest(requestId: String): IncomingRequest.AuthorizedRequest?
Comment on lines -26 to -30
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice that you removed those 👍

fun getRequest(requestId: String): IncomingRequest?

fun removeAll()

fun getAmountOfRequests(): Int

suspend fun requestDeferred(requestId: String)

fun consumeBufferedRequest(): IncomingRequest?

fun setBufferedRequest(request: IncomingRequest)
}

class IncomingRequestRepositoryImpl @Inject constructor() : IncomingRequestRepository {
@Suppress("TooManyFunctions")
class IncomingRequestRepositoryImpl @Inject constructor(
private val appEventBus: AppEventBus
) : IncomingRequestRepository {

private val requestQueue = LinkedList<QueueItem>()

Expand All @@ -52,6 +61,19 @@ class IncomingRequestRepositoryImpl @Inject constructor() : IncomingRequestRepos

private val mutex = Mutex()

/**
* Request that can come in via deep link before wallet is setup.
*/
Comment on lines +64 to +66
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌🏽

private var bufferedRequest: IncomingRequest? = null

override fun setBufferedRequest(request: IncomingRequest) {
bufferedRequest = request
}

override fun consumeBufferedRequest(): IncomingRequest? {
return bufferedRequest?.also { bufferedRequest = null }
}

override suspend fun add(incomingRequest: IncomingRequest) {
mutex.withLock {
val requestItem = QueueItem.RequestItem(incomingRequest)
Expand All @@ -68,25 +90,71 @@ class IncomingRequestRepositoryImpl @Inject constructor() : IncomingRequestRepos
}
}

/**
* There are two path of execution when using this method:
* - there is high priority screen in the queue, so the incoming request is added below it,
* taking priority over requests currently in the queue
* - there is no high priority screen: request is added at the top of queue and if there other request currently handled,
* we send a defer event for it so that UI can react and defer handling, without removing it from the queue.
* Deferred request will be handled again when top priority one handling completes
*/
override suspend fun addPriorityRequest(incomingRequest: IncomingRequest) {
mutex.withLock {
requestQueue.addFirst(QueueItem.RequestItem(incomingRequest))
val currentRequest = _currentRequestToHandle.value
val handlingPaused = requestQueue.contains(QueueItem.HighPriorityScreen)
when {
currentRequest != null -> {
Timber.d("🗂 Deferring request with id ${currentRequest.interactionId}")
appEventBus.sendEvent(AppEvent.DeferRequestHandling(currentRequest.interactionId))
}

else -> {
if (!handlingPaused) {
handleNextRequest()
}
}
}
}
}

override suspend fun requestHandled(requestId: String) {
mutex.withLock {
requestQueue.removeIf { it is QueueItem.RequestItem && it.incomingRequest.interactionId.toString() == requestId }
requestQueue.removeIf { it is QueueItem.RequestItem && it.incomingRequest.interactionId == requestId }
clearCurrent(requestId)
handleNextRequest()
Timber.d("🗂 request $requestId handled so size of list is now: ${getAmountOfRequests()}")
}
}

override suspend fun requestDeferred(requestId: String) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not actually deferred here? This is like a yield action right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is deferred, but this method works in tandem with addMobileConnectRequest. When you use the latter, new request is put in front, and calling requestDeferred is simply taking next request to handle without removing current from the queue. I couldn't do both those actions in one method as there are user interactions in between.

mutex.withLock {
clearCurrent(requestId)
handleNextRequest()
Timber.d("🗂 request $requestId handled so size of list is now: ${getAmountOfRequests()}")
}
}

private suspend fun clearCurrent(requestId: String) {
if (_currentRequestToHandle.value?.interactionId == requestId) {
_currentRequestToHandle.emit(null)
}
}

override suspend fun pauseIncomingRequests() {
mutex.withLock {
// If the queue knows about any high priority item, no need to add it again
if (requestQueue.contains(QueueItem.HighPriorityScreen)) {
return
}

// Put high priority item below any internal request
val topQueueItem = requestQueue.peekFirst()
if (topQueueItem is QueueItem.RequestItem && topQueueItem.incomingRequest.isInternal) {
requestQueue.add(1, QueueItem.HighPriorityScreen)
// Put high priority item below any internal request and mobile connect requests
val index = requestQueue.indexOfFirst {
val item = it as? QueueItem.RequestItem
item != null && !item.incomingRequest.isInternal && !item.incomingRequest.isMobileConnectRequest
}
if (index != -1) {
requestQueue.add(index, QueueItem.HighPriorityScreen)
} else {
requestQueue.addFirst(QueueItem.HighPriorityScreen)
}
Expand All @@ -104,37 +172,14 @@ class IncomingRequestRepositoryImpl @Inject constructor() : IncomingRequestRepos
}
}

override fun getUnauthorizedRequest(requestId: String): IncomingRequest.UnauthorizedRequest? {
override fun getRequest(requestId: String): IncomingRequest? {
val queueItem = requestQueue.find {
it is QueueItem.RequestItem && it.incomingRequest.interactionId.toString() == requestId &&
it.incomingRequest is IncomingRequest.UnauthorizedRequest
it is QueueItem.RequestItem && it.incomingRequest.interactionId == requestId
}
if (queueItem == null) {
Timber.w("Unauthorized request with id $requestId is null")
Timber.w("Request with id $requestId is null")
}
return (queueItem as? QueueItem.RequestItem)?.incomingRequest as? IncomingRequest.UnauthorizedRequest
}

override fun getTransactionWriteRequest(requestId: String): IncomingRequest.TransactionRequest? {
val queueItem = requestQueue.find {
it is QueueItem.RequestItem && it.incomingRequest.interactionId.toString() == requestId &&
it.incomingRequest is IncomingRequest.TransactionRequest
}
if (queueItem == null) {
Timber.w("Transaction request with id $requestId is null")
}
return (queueItem as? QueueItem.RequestItem)?.incomingRequest as? IncomingRequest.TransactionRequest
}

override fun getAuthorizedRequest(requestId: String): IncomingRequest.AuthorizedRequest? {
val queueItem = requestQueue.find {
it is QueueItem.RequestItem && it.incomingRequest.interactionId.toString() == requestId &&
it.incomingRequest is IncomingRequest.AuthorizedRequest
}
if (queueItem == null) {
Timber.w("Authorized request with id $requestId is null")
}
return (queueItem as? QueueItem.RequestItem)?.incomingRequest as? IncomingRequest.AuthorizedRequest
return (queueItem as? QueueItem.RequestItem)?.incomingRequest
}

override fun removeAll() {
Expand All @@ -149,14 +194,13 @@ class IncomingRequestRepositoryImpl @Inject constructor() : IncomingRequestRepos
// In order to emit an incoming request, the topmost item should be
// a. An incoming request
// b. It should not be the same as the one being handled already
if (nextRequest is QueueItem.RequestItem && _currentRequestToHandle.value != nextRequest) {
if (nextRequest is QueueItem.RequestItem && _currentRequestToHandle.value != nextRequest.incomingRequest) {
_currentRequestToHandle.emit(nextRequest.incomingRequest)
}
}

private sealed interface QueueItem {
data object HighPriorityScreen : QueueItem

data class RequestItem(val incomingRequest: IncomingRequest) : QueueItem
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ sealed interface IncomingMessage {

val value: String

data class RadixMobileConnectRemoteSession(val id: String) : RemoteEntityID {
data class RadixMobileConnectRemoteSession(val id: String, val originVerificationUrl: String? = null) : RemoteEntityID {
override val value: String
get() = id

val needOriginVerification: Boolean = originVerificationUrl != null
}

data class ConnectorId(val id: String) : RemoteEntityID {
Expand All @@ -50,6 +52,9 @@ sealed interface IncomingMessage {
val isMobileConnectRequest: Boolean
get() = remoteEntityId is RemoteEntityID.RadixMobileConnectRemoteSession

val needVerification: Boolean
get() = (remoteEntityId as? RemoteEntityID.RadixMobileConnectRemoteSession)?.needOriginVerification == true

val blockUntilComplete: Boolean
get() {
return metadata.blockUntilCompleted
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import com.radixdlt.sargon.AccountAddress
import com.radixdlt.sargon.DappWalletInteractionErrorType
import com.radixdlt.sargon.extensions.init
import rdx.works.core.domain.DApp
import rdx.works.core.sargon.currentGateway
import rdx.works.core.then
import rdx.works.profile.domain.GetProfileUseCase
import javax.inject.Inject
Expand All @@ -24,14 +25,26 @@ class VerifyDAppUseCase @Inject constructor(
) {

suspend operator fun invoke(request: IncomingRequest): Result<Boolean> {
val networkId = getProfileUseCase().currentGateway.network.id
if (networkId != request.metadata.networkId) {
respondToIncomingRequestUseCase.respondWithFailure(
request = request,
error = DappWalletInteractionErrorType.INVALID_REQUEST
)
return Result.failure(
RadixWalletException.DappRequestException.WrongNetwork(
currentNetworkId = networkId,
requestNetworkId = request.metadata.networkId
)
)
}
val dAppDefinitionAddress = runCatching { AccountAddress.init(request.metadata.dAppDefinitionAddress) }.getOrElse {
respondToIncomingRequestUseCase.respondWithFailure(
request = request,
error = DappWalletInteractionErrorType.INVALID_REQUEST
)
return Result.failure(RadixWalletException.DappRequestException.InvalidRequest)
}

val developerMode = getProfileUseCase().appPreferences.security.isDeveloperModeEnabled
return if (developerMode) {
Result.success(true)
Expand Down
Loading
Loading