From 419475c52973320045a86c20026adf08f9bd27c4 Mon Sep 17 00:00:00 2001 From: John Johnson Date: Thu, 23 Mar 2023 15:49:44 -0400 Subject: [PATCH] Update wallet suggestions controller to no longer use a broadcast for suggestions The service added in http://ag/22168927 will listen for updates from PCC and give those to the controller Bug: b/237409756 Test: atest com.android.systemui.wallet.controller.WalletContextualSuggestionsControllerTest Test: Manually verified this works with child change http://ag/22207162 Change-Id: I750a1461750c7a63d2f67e474804d25cd7d9a7dd --- .../WalletContextualSuggestionsController.kt | 136 ++++++++++------- ...lletContextualSuggestionsControllerTest.kt | 138 +++++++++--------- 2 files changed, 155 insertions(+), 119 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/wallet/controller/WalletContextualSuggestionsController.kt b/packages/SystemUI/src/com/android/systemui/wallet/controller/WalletContextualSuggestionsController.kt index 7b8235acb0a7f..518f5a774d7f2 100644 --- a/packages/SystemUI/src/com/android/systemui/wallet/controller/WalletContextualSuggestionsController.kt +++ b/packages/SystemUI/src/com/android/systemui/wallet/controller/WalletContextualSuggestionsController.kt @@ -16,8 +16,7 @@ package com.android.systemui.wallet.controller -import android.Manifest -import android.content.Context +import android.content.Intent import android.content.IntentFilter import android.service.quickaccesswallet.GetWalletCardsError import android.service.quickaccesswallet.GetWalletCardsResponse @@ -32,13 +31,21 @@ import com.android.systemui.flags.FeatureFlags import com.android.systemui.flags.Flags import javax.inject.Inject import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.emptyFlow -import kotlinx.coroutines.flow.shareIn +import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.stateIn +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch +@OptIn(ExperimentalCoroutinesApi::class) @SysUISingleton class WalletContextualSuggestionsController @Inject @@ -48,68 +55,99 @@ constructor( broadcastDispatcher: BroadcastDispatcher, featureFlags: FeatureFlags ) { + private val cardsReceivedCallbacks: MutableSet<(List) -> Unit> = mutableSetOf() + private val allWalletCards: Flow> = if (featureFlags.isEnabled(Flags.ENABLE_WALLET_CONTEXTUAL_LOYALTY_CARDS)) { - conflatedCallbackFlow { - val callback = - object : QuickAccessWalletClient.OnWalletCardsRetrievedCallback { - override fun onWalletCardsRetrieved(response: GetWalletCardsResponse) { - trySendWithFailureLogging(response.walletCards, TAG) - } + // TODO(b/237409756) determine if we should debounce this so we don't call the service + // too frequently. Also check if the list actually changed before calling callbacks. + broadcastDispatcher + .broadcastFlow(IntentFilter(Intent.ACTION_SCREEN_ON)) + .flatMapLatest { + conflatedCallbackFlow { + val callback = + object : QuickAccessWalletClient.OnWalletCardsRetrievedCallback { + override fun onWalletCardsRetrieved( + response: GetWalletCardsResponse + ) { + trySendWithFailureLogging(response.walletCards, TAG) + } - override fun onWalletCardRetrievalError(error: GetWalletCardsError) { - trySendWithFailureLogging(emptyList(), TAG) + override fun onWalletCardRetrievalError( + error: GetWalletCardsError + ) { + trySendWithFailureLogging(emptyList(), TAG) + } + } + + walletController.setupWalletChangeObservers( + callback, + QuickAccessWalletController.WalletChangeEvent.WALLET_PREFERENCE_CHANGE, + QuickAccessWalletController.WalletChangeEvent.DEFAULT_PAYMENT_APP_CHANGE + ) + walletController.updateWalletPreference() + walletController.queryWalletCards(callback) + + awaitClose { + walletController.unregisterWalletChangeObservers( + QuickAccessWalletController.WalletChangeEvent + .WALLET_PREFERENCE_CHANGE, + QuickAccessWalletController.WalletChangeEvent + .DEFAULT_PAYMENT_APP_CHANGE + ) } } - - walletController.setupWalletChangeObservers( - callback, - QuickAccessWalletController.WalletChangeEvent.WALLET_PREFERENCE_CHANGE, - QuickAccessWalletController.WalletChangeEvent.DEFAULT_PAYMENT_APP_CHANGE + } + .onEach { notifyCallbacks(it) } + .stateIn( + applicationCoroutineScope, + // Needs to be done eagerly since we need to notify callbacks even if there are + // no subscribers + SharingStarted.Eagerly, + emptyList() ) - walletController.updateWalletPreference() - walletController.queryWalletCards(callback) - - awaitClose { - walletController.unregisterWalletChangeObservers( - QuickAccessWalletController.WalletChangeEvent.WALLET_PREFERENCE_CHANGE, - QuickAccessWalletController.WalletChangeEvent.DEFAULT_PAYMENT_APP_CHANGE - ) - } - } } else { emptyFlow() } - private val contextualSuggestionsCardIds: Flow> = - if (featureFlags.isEnabled(Flags.ENABLE_WALLET_CONTEXTUAL_LOYALTY_CARDS)) { - broadcastDispatcher.broadcastFlow( - filter = IntentFilter(ACTION_UPDATE_WALLET_CONTEXTUAL_SUGGESTIONS), - permission = Manifest.permission.BIND_QUICK_ACCESS_WALLET_SERVICE, - flags = Context.RECEIVER_EXPORTED - ) { intent, _ -> - if (intent.hasExtra(UPDATE_CARD_IDS_EXTRA)) { - intent.getStringArrayListExtra(UPDATE_CARD_IDS_EXTRA).toSet() - } else { - emptySet() - } - } - } else { - emptyFlow() - } + private val _suggestionCardIds: MutableStateFlow> = MutableStateFlow(emptySet()) + private val contextualSuggestionsCardIds: Flow> = _suggestionCardIds.asStateFlow() val contextualSuggestionCards: Flow> = combine(allWalletCards, contextualSuggestionsCardIds) { cards, ids -> - cards.filter { card -> ids.contains(card.cardId) } + val ret = + cards.filter { card -> + card.cardType == WalletCard.CARD_TYPE_NON_PAYMENT && + ids.contains(card.cardId) + } + ret } - .shareIn(applicationCoroutineScope, replay = 1, started = SharingStarted.Eagerly) + .stateIn(applicationCoroutineScope, SharingStarted.WhileSubscribed(), emptyList()) + + /** When called, {@link contextualSuggestionCards} will be updated to be for these IDs. */ + fun setSuggestionCardIds(cardIds: Set) { + _suggestionCardIds.update { _ -> cardIds } + } + + /** Register callback to be called when a new list of cards is fetched. */ + fun registerWalletCardsReceivedCallback(callback: (List) -> Unit) { + cardsReceivedCallbacks.add(callback) + } + + /** Unregister callback to be called when a new list of cards is fetched. */ + fun unregisterWalletCardsReceivedCallback(callback: (List) -> Unit) { + cardsReceivedCallbacks.remove(callback) + } + + private fun notifyCallbacks(cards: List) { + applicationCoroutineScope.launch { + cardsReceivedCallbacks.onEach { callback -> + callback(cards.filter { card -> card.cardType == WalletCard.CARD_TYPE_NON_PAYMENT }) + } + } + } companion object { - private const val ACTION_UPDATE_WALLET_CONTEXTUAL_SUGGESTIONS = - "com.android.systemui.wallet.UPDATE_CONTEXTUAL_SUGGESTIONS" - - private const val UPDATE_CARD_IDS_EXTRA = "cardIds" - private const val TAG = "WalletSuggestions" } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/wallet/controller/WalletContextualSuggestionsControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/wallet/controller/WalletContextualSuggestionsControllerTest.kt index b52786178e717..9bd3a799350d4 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/wallet/controller/WalletContextualSuggestionsControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/wallet/controller/WalletContextualSuggestionsControllerTest.kt @@ -32,9 +32,9 @@ import com.android.systemui.util.mockito.any import com.android.systemui.util.mockito.capture import com.android.systemui.util.mockito.eq import com.android.systemui.util.mockito.mock +import com.android.systemui.util.mockito.nullable import com.android.systemui.util.mockito.whenever import com.google.common.truth.Truth.assertThat -import java.util.ArrayList import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runCurrent @@ -46,7 +46,7 @@ import org.junit.runners.JUnit4 import org.mockito.ArgumentCaptor import org.mockito.Captor import org.mockito.Mock -import org.mockito.Mockito.isNull +import org.mockito.Mockito.anyInt import org.mockito.Mockito.never import org.mockito.Mockito.verify import org.mockito.MockitoAnnotations @@ -66,12 +66,14 @@ class WalletContextualSuggestionsControllerTest : SysuiTestCase() { fun setUp() { MockitoAnnotations.initMocks(this) + whenever(broadcastDispatcher.broadcastFlow(any(), nullable(), anyInt(), nullable())) + .thenCallRealMethod() whenever( - broadcastDispatcher.broadcastFlow?>( - any(), - isNull(), - any(), + broadcastDispatcher.broadcastFlow( any(), + nullable(), + anyInt(), + nullable(), any() ) ) @@ -81,95 +83,85 @@ class WalletContextualSuggestionsControllerTest : SysuiTestCase() { .thenReturn(true) whenever(CARD_1.cardId).thenReturn(ID_1) + whenever(CARD_1.cardType).thenReturn(WalletCard.CARD_TYPE_NON_PAYMENT) whenever(CARD_2.cardId).thenReturn(ID_2) + whenever(CARD_2.cardType).thenReturn(WalletCard.CARD_TYPE_NON_PAYMENT) whenever(CARD_3.cardId).thenReturn(ID_3) + whenever(CARD_3.cardType).thenReturn(WalletCard.CARD_TYPE_NON_PAYMENT) + whenever(PAYMENT_CARD.cardId).thenReturn(PAYMENT_ID) + whenever(PAYMENT_CARD.cardType).thenReturn(WalletCard.CARD_TYPE_PAYMENT) } @Test - fun `state - has wallet cards - received contextual cards`() = runTest { - setUpWalletClient(listOf(CARD_1, CARD_2)) - val latest = - collectLastValue( - createWalletContextualSuggestionsController(backgroundScope) - .contextualSuggestionCards, - ) + fun `state - has wallet cards- callbacks called`() = runTest { + setUpWalletClient(listOf(CARD_1, CARD_2, PAYMENT_CARD)) + val controller = createWalletContextualSuggestionsController(backgroundScope) + var latest1 = emptyList() + var latest2 = emptyList() + val callback1: (List) -> Unit = { latest1 = it } + val callback2: (List) -> Unit = { latest2 = it } runCurrent() - verifyRegistered() - broadcastReceiver.value.onReceive( - mockContext, - createContextualCardsIntent(listOf(ID_1, ID_2)) - ) + controller.registerWalletCardsReceivedCallback(callback1) + controller.registerWalletCardsReceivedCallback(callback2) + controller.unregisterWalletCardsReceivedCallback(callback2) + runCurrent() + verifyBroadcastReceiverRegistered() + turnScreenOn() + runCurrent() - assertThat(latest()).containsExactly(CARD_1, CARD_2) + assertThat(latest1).containsExactly(CARD_1, CARD_2) + assertThat(latest2).isEmpty() } @Test - fun `state - no wallet cards - received contextual cards`() = runTest { + fun `state - no wallet cards - set suggestion cards`() = runTest { setUpWalletClient(emptyList()) + val controller = createWalletContextualSuggestionsController(backgroundScope) val latest = collectLastValue( - createWalletContextualSuggestionsController(backgroundScope) - .contextualSuggestionCards, + controller.contextualSuggestionCards, ) runCurrent() - verifyRegistered() - broadcastReceiver.value.onReceive( - mockContext, - createContextualCardsIntent(listOf(ID_1, ID_2)) - ) + verifyBroadcastReceiverRegistered() + turnScreenOn() + controller.setSuggestionCardIds(setOf(ID_1, ID_2)) assertThat(latest()).isEmpty() } @Test - fun `state - has wallet cards - no contextual cards`() = runTest { - setUpWalletClient(listOf(CARD_1, CARD_2)) + fun `state - has wallet cards - set and update suggestion cards`() = runTest { + setUpWalletClient(listOf(CARD_1, CARD_2, PAYMENT_CARD)) + val controller = createWalletContextualSuggestionsController(backgroundScope) val latest = collectLastValue( - createWalletContextualSuggestionsController(backgroundScope) - .contextualSuggestionCards, + controller.contextualSuggestionCards, ) runCurrent() - verifyRegistered() - broadcastReceiver.value.onReceive(mockContext, createContextualCardsIntent(emptyList())) + verifyBroadcastReceiverRegistered() + turnScreenOn() + controller.setSuggestionCardIds(setOf(ID_1, ID_2)) + assertThat(latest()).containsExactly(CARD_1, CARD_2) + controller.setSuggestionCardIds(emptySet()) assertThat(latest()).isEmpty() } @Test fun `state - wallet cards error`() = runTest { setUpWalletClient(shouldFail = true) + val controller = createWalletContextualSuggestionsController(backgroundScope) val latest = collectLastValue( - createWalletContextualSuggestionsController(backgroundScope) - .contextualSuggestionCards, + controller.contextualSuggestionCards, ) runCurrent() - verifyRegistered() - broadcastReceiver.value.onReceive( - mockContext, - createContextualCardsIntent(listOf(ID_1, ID_2)) - ) - - assertThat(latest()).isEmpty() - } - - @Test - fun `state - no contextual cards extra`() = runTest { - setUpWalletClient(listOf(CARD_1, CARD_2)) - val latest = - collectLastValue( - createWalletContextualSuggestionsController(backgroundScope) - .contextualSuggestionCards, - ) - - runCurrent() - verifyRegistered() - broadcastReceiver.value.onReceive(mockContext, Intent(INTENT_NAME)) + verifyBroadcastReceiverRegistered() + controller.setSuggestionCardIds(setOf(ID_1, ID_2)) assertThat(latest()).isEmpty() } @@ -178,16 +170,18 @@ class WalletContextualSuggestionsControllerTest : SysuiTestCase() { fun `state - has wallet cards - received contextual cards - feature disabled`() = runTest { whenever(featureFlags.isEnabled(eq(Flags.ENABLE_WALLET_CONTEXTUAL_LOYALTY_CARDS))) .thenReturn(false) - setUpWalletClient(listOf(CARD_1, CARD_2)) + setUpWalletClient(listOf(CARD_1, CARD_2, PAYMENT_CARD)) + val controller = createWalletContextualSuggestionsController(backgroundScope) val latest = collectLastValue( - createWalletContextualSuggestionsController(backgroundScope) - .contextualSuggestionCards, + controller.contextualSuggestionCards, ) runCurrent() - verify(broadcastDispatcher, never()).broadcastFlow(any(), isNull(), any(), any()) - assertThat(latest()).isNull() + verify(broadcastDispatcher, never()).broadcastFlow(any(), nullable(), anyInt(), nullable()) + controller.setSuggestionCardIds(setOf(ID_1, ID_2)) + + assertThat(latest()).isEmpty() } private fun createWalletContextualSuggestionsController( @@ -201,17 +195,20 @@ class WalletContextualSuggestionsControllerTest : SysuiTestCase() { ) } - private fun verifyRegistered() { + private fun verifyBroadcastReceiverRegistered() { verify(broadcastDispatcher) - .registerReceiver(capture(broadcastReceiver), any(), isNull(), isNull(), any(), any()) + .registerReceiver( + capture(broadcastReceiver), + any(), + nullable(), + nullable(), + anyInt(), + nullable() + ) } - private fun createContextualCardsIntent( - ids: List = emptyList(), - ): Intent { - val intent = Intent(INTENT_NAME) - intent.putStringArrayListExtra("cardIds", ArrayList(ids)) - return intent + private fun turnScreenOn() { + broadcastReceiver.value.onReceive(mockContext, Intent(Intent.ACTION_SCREEN_ON)) } private fun setUpWalletClient( @@ -238,6 +235,7 @@ class WalletContextualSuggestionsControllerTest : SysuiTestCase() { private val CARD_2: WalletCard = mock() private const val ID_3: String = "789" private val CARD_3: WalletCard = mock() - private val INTENT_NAME: String = "WalletSuggestionsIntent" + private const val PAYMENT_ID: String = "payment" + private val PAYMENT_CARD: WalletCard = mock() } }