From fafa0a470132f0c7a1b8f036f9ea0a3f18a1ab13 Mon Sep 17 00:00:00 2001 From: dani Date: Thu, 21 Mar 2024 08:29:51 +0200 Subject: [PATCH] feat(health-sdk): Addressed code review comments IPC-186 --- .../sdk/paymentcomponent/PaymentComponent.kt | 4 +- .../sdk/paymentprovider/PaymentProviderApp.kt | 22 +-- .../health/sdk/util/extensions/Context.kt | 30 ++++ .../paymentComponent/PaymentComponentTest.kt | 149 +++++++++++++++--- .../PaymentComponentViewTest.kt | 5 +- 5 files changed, 164 insertions(+), 46 deletions(-) create mode 100644 health-sdk/sdk/src/main/java/net/gini/android/health/sdk/util/extensions/Context.kt diff --git a/health-sdk/sdk/src/main/java/net/gini/android/health/sdk/paymentcomponent/PaymentComponent.kt b/health-sdk/sdk/src/main/java/net/gini/android/health/sdk/paymentcomponent/PaymentComponent.kt index 6f4bf1f23a..c5e13b9092 100644 --- a/health-sdk/sdk/src/main/java/net/gini/android/health/sdk/paymentcomponent/PaymentComponent.kt +++ b/health-sdk/sdk/src/main/java/net/gini/android/health/sdk/paymentcomponent/PaymentComponent.kt @@ -1,6 +1,7 @@ package net.gini.android.health.sdk.paymentcomponent import android.content.Context +import androidx.annotation.VisibleForTesting import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow @@ -22,7 +23,8 @@ class PaymentComponent(private val context: Context, private val giniHealth: Gin MutableStateFlow(SelectedPaymentProviderAppState.NothingSelected) val selectedPaymentProviderAppFlow: StateFlow = _selectedPaymentProviderAppFlow.asStateFlow() - private val paymentComponentPreferences = PaymentComponentPreferences(context) + @VisibleForTesting + internal val paymentComponentPreferences = PaymentComponentPreferences(context) var listener: Listener? = null diff --git a/health-sdk/sdk/src/main/java/net/gini/android/health/sdk/paymentprovider/PaymentProviderApp.kt b/health-sdk/sdk/src/main/java/net/gini/android/health/sdk/paymentprovider/PaymentProviderApp.kt index 64d1c0017e..5c14a2d9db 100644 --- a/health-sdk/sdk/src/main/java/net/gini/android/health/sdk/paymentprovider/PaymentProviderApp.kt +++ b/health-sdk/sdk/src/main/java/net/gini/android/health/sdk/paymentprovider/PaymentProviderApp.kt @@ -5,15 +5,12 @@ import android.content.Context import android.content.Intent import android.content.pm.PackageManager import android.content.pm.ResolveInfo -import android.graphics.Bitmap -import android.graphics.BitmapFactory import android.graphics.Color import android.graphics.drawable.BitmapDrawable -import android.graphics.drawable.Drawable import android.net.Uri -import android.util.TypedValue import androidx.annotation.ColorInt import net.gini.android.health.api.models.PaymentProvider +import net.gini.android.health.sdk.util.extensions.generateBitmapDrawableIcon import org.slf4j.LoggerFactory internal const val Scheme = "ginipay" // It has to match the scheme in query tag in manifest @@ -122,22 +119,7 @@ data class PaymentProviderApp( } return PaymentProviderApp( name = paymentProvider.name, - icon = BitmapFactory.decodeByteArray(paymentProvider.icon, 0, paymentProvider.icon.size) - ?.let { bitmap -> - val iconSizePx = TypedValue.applyDimension( - TypedValue.COMPLEX_UNIT_DIP, - ICON_SIZE, - context.resources.displayMetrics - ).toInt() - val scaledBitamp = Bitmap.createScaledBitmap( - bitmap, - iconSizePx, - iconSizePx, - true - ) - bitmap.recycle() - BitmapDrawable(context.resources, scaledBitamp) - }, + icon = context.generateBitmapDrawableIcon(paymentProvider.icon, paymentProvider.icon.size), colors = PaymentProviderAppColors( backgroundColor = Color.parseColor("#${paymentProvider.colors.backgroundColorRGBHex}"), textColor = Color.parseColor("#${paymentProvider.colors.textColoRGBHex}") diff --git a/health-sdk/sdk/src/main/java/net/gini/android/health/sdk/util/extensions/Context.kt b/health-sdk/sdk/src/main/java/net/gini/android/health/sdk/util/extensions/Context.kt new file mode 100644 index 0000000000..cad7ed55f9 --- /dev/null +++ b/health-sdk/sdk/src/main/java/net/gini/android/health/sdk/util/extensions/Context.kt @@ -0,0 +1,30 @@ +package net.gini.android.health.sdk.util.extensions + +import android.content.Context +import android.graphics.Bitmap +import android.graphics.BitmapFactory +import android.graphics.drawable.BitmapDrawable +import android.util.TypedValue +import androidx.annotation.VisibleForTesting +import net.gini.android.health.sdk.paymentprovider.PaymentProviderApp + +// In a future refactoring we can split extensions into files according to what component they extend +@VisibleForTesting +internal fun Context.generateBitmapDrawableIcon(icon: ByteArray, iconSize: Int): BitmapDrawable? { + return BitmapFactory.decodeByteArray(icon, 0, iconSize) + ?.let { bitmap -> + val iconSizePx = TypedValue.applyDimension( + TypedValue.COMPLEX_UNIT_DIP, + PaymentProviderApp.ICON_SIZE, + this.resources.displayMetrics + ).toInt() + val scaledBitmap = Bitmap.createScaledBitmap( + bitmap, + iconSizePx, + iconSizePx, + true + ) + bitmap.recycle() + BitmapDrawable(this.resources, scaledBitmap) + } +} \ No newline at end of file diff --git a/health-sdk/sdk/src/test/java/net/gini/android/health/sdk/paymentComponent/PaymentComponentTest.kt b/health-sdk/sdk/src/test/java/net/gini/android/health/sdk/paymentComponent/PaymentComponentTest.kt index 8b33957697..b019ea6986 100644 --- a/health-sdk/sdk/src/test/java/net/gini/android/health/sdk/paymentComponent/PaymentComponentTest.kt +++ b/health-sdk/sdk/src/test/java/net/gini/android/health/sdk/paymentComponent/PaymentComponentTest.kt @@ -2,14 +2,17 @@ package net.gini.android.health.sdk.paymentComponent import android.content.Context import android.content.pm.PackageManager -import android.util.DisplayMetrics -import androidx.test.core.app.ApplicationProvider +import android.content.res.Resources +import androidx.test.core.app.ApplicationProvider.getApplicationContext import androidx.test.ext.junit.runners.AndroidJUnit4 import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import io.mockk.coEvery import io.mockk.every import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.spyk +import io.mockk.unmockkAll import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.runTest @@ -19,19 +22,21 @@ import net.gini.android.health.api.HealthApiDocumentManager import net.gini.android.health.api.models.PaymentProvider import net.gini.android.health.sdk.GiniHealth import net.gini.android.health.sdk.paymentcomponent.PaymentComponent -import net.gini.android.health.sdk.paymentcomponent.PaymentComponentPreferences import net.gini.android.health.sdk.paymentcomponent.PaymentProviderAppsState import net.gini.android.health.sdk.paymentcomponent.SelectedPaymentProviderAppState import net.gini.android.health.sdk.paymentprovider.PaymentProviderApp +import net.gini.android.health.sdk.paymentprovider.PaymentProviderAppColors +import net.gini.android.health.sdk.paymentprovider.getPaymentProviderApps import net.gini.android.health.sdk.review.ReviewConfiguration import net.gini.android.health.sdk.review.ReviewFragment import net.gini.android.health.sdk.test.ViewModelTestCoroutineRule +import net.gini.android.health.sdk.util.extensions.generateBitmapDrawableIcon +import org.junit.After import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith - @ExperimentalCoroutinesApi @RunWith(AndroidJUnit4::class) class PaymentComponentTest { @@ -39,9 +44,8 @@ class PaymentComponentTest { @get:Rule val testCoroutineRule = ViewModelTestCoroutineRule() - private lateinit var context: Context - private lateinit var packageManager: PackageManager - private lateinit var giniHealth: GiniHealth + private var context: Context? = null + private var giniHealth: GiniHealth? = null private val giniHealthAPI: GiniHealthAPI = mockk(relaxed = true) { GiniHealthAPI::class.java } private val documentManager: HealthApiDocumentManager = mockk { HealthApiDocumentManager::class.java } @@ -84,7 +88,7 @@ class PaymentComponentTest { playStoreUrl = "" ) - private val notInstalledPaymentProvider = PaymentProvider( + private val noPlayStoreUrlPaymentProvider = PaymentProvider( id = "payment provider id 3", name = "payment provider name", packageName = "net.gini.android.bank.exampleapp3", @@ -100,17 +104,50 @@ class PaymentComponentTest { fun setUp() { every { giniHealthAPI.documentManager } returns documentManager giniHealth = GiniHealth(giniHealthAPI) - context = ApplicationProvider.getApplicationContext() - packageManager = mockk(relaxed = true) + context = getApplicationContext() + } + + @After + fun tearDown() { + giniHealth = null + context = null + unmockkAll() + } + + private fun createMockedContextAndSetDependencies(paymentProviderList: List, paymentProviderAppsList: List): Context { + val privateContext: Context = mockk() + val resources: Resources = spyk(context!!.resources) + val packageManager: PackageManager = mockk(relaxed = true) + + mockkStatic(Context::generateBitmapDrawableIcon) + mockkStatic(PackageManager::getPaymentProviderApps) + every { privateContext.applicationContext } returns context + every { privateContext.packageManager } returns packageManager + every { privateContext.resources } returns resources + every { privateContext.generateBitmapDrawableIcon(any(), any()) } returns null + every { packageManager.getPaymentProviderApps(paymentProviderList, privateContext) } returns paymentProviderAppsList + + return privateContext } + private fun buildPaymentProviderApp(paymentProvider: PaymentProvider, isInstalled: Boolean) = PaymentProviderApp( + name = paymentProvider.name, + icon = null, + colors = PaymentProviderAppColors( + backgroundColor = 0, + textColor = 0 + ), + paymentProvider = paymentProvider, + installedPaymentProviderApp = if (isInstalled) mockk(relaxed = true) else null + ) + @Test fun `emits error when it cannot load payment providers`() = runTest { // Given coEvery { documentManager.getPaymentProviders() } returns Resource.Error() // When - val paymentComponent = PaymentComponent(context, giniHealth) + val paymentComponent = PaymentComponent(context!!, giniHealth!!) paymentComponent.loadPaymentProviderApps() // Then @@ -125,7 +162,7 @@ class PaymentComponentTest { coEvery { documentManager.getPaymentProviders() } returns Resource.Cancelled() // When - val paymentComponent = PaymentComponent(context, giniHealth) + val paymentComponent = PaymentComponent(context!!, giniHealth!!) paymentComponent.loadPaymentProviderApps() // Then @@ -146,7 +183,7 @@ class PaymentComponentTest { coEvery { documentManager.getPaymentProviders() } returns Resource.Success(paymentProviderList) // When - val paymentComponent = PaymentComponent(context, giniHealth) + val paymentComponent = PaymentComponent(context!!, giniHealth!!) paymentComponent.loadPaymentProviderApps() // Then @@ -160,19 +197,87 @@ class PaymentComponentTest { } @Test - fun `emits only installed payment providers`() = runTest { + fun `doesn't emit payment providers which are not installed and which don't have a Play Store url`() = runTest { // Given val paymentProviderList = listOf( paymentProvider, paymentProvider1, paymentProvider2, - notInstalledPaymentProvider + noPlayStoreUrlPaymentProvider ) coEvery { documentManager.getPaymentProviders() } returns Resource.Success(paymentProviderList) + val paymentProviderAppList = listOf( + buildPaymentProviderApp(paymentProvider, true), + buildPaymentProviderApp(paymentProvider1, true), + buildPaymentProviderApp(paymentProvider2, true), + buildPaymentProviderApp(noPlayStoreUrlPaymentProvider, false) + ) + val mockedContext = createMockedContextAndSetDependencies(paymentProviderList, paymentProviderAppList) + //When - val paymentComponent = PaymentComponent(context, giniHealth) + val paymentComponent = PaymentComponent(mockedContext, giniHealth!!) + paymentComponent.loadPaymentProviderApps() + + // Then + paymentComponent.paymentProviderAppsFlow.test { + val validation = awaitItem() + assertThat(validation).isInstanceOf(PaymentProviderAppsState.Success::class.java) + assertThat((validation as PaymentProviderAppsState.Success).paymentProviderApps.size).isEqualTo(3) + + cancelAndConsumeRemainingEvents() + } + } + + @Test + fun `emits payment provider if it's not installed but has Play Store url`() = runTest { + // Given + val paymentProviderList = listOf( + paymentProvider + ) + + coEvery { documentManager.getPaymentProviders() } returns Resource.Success(paymentProviderList) + + val paymentProviderAppList = listOf(buildPaymentProviderApp(paymentProvider, false)) + val mockedContext = createMockedContextAndSetDependencies(paymentProviderList, paymentProviderAppList) + + //When + val paymentComponent = PaymentComponent(mockedContext, giniHealth!!) + paymentComponent.loadPaymentProviderApps() + + // Then + paymentComponent.paymentProviderAppsFlow.test { + val validation = awaitItem() + assertThat(validation).isInstanceOf(PaymentProviderAppsState.Success::class.java) + assertThat((validation as PaymentProviderAppsState.Success).paymentProviderApps.size).isEqualTo(1) + + cancelAndConsumeRemainingEvents() + } + } + + @Test + fun `emits payment provider if it has Play Store URL and is installed`() = runTest { + // Given + val paymentProviderList = listOf( + paymentProvider, + paymentProvider1, + paymentProvider2, + noPlayStoreUrlPaymentProvider + ) + + coEvery { documentManager.getPaymentProviders() } returns Resource.Success(paymentProviderList) + + val paymentProviderAppsList = listOf( + buildPaymentProviderApp(paymentProvider, true), + buildPaymentProviderApp(paymentProvider1, true), + buildPaymentProviderApp(paymentProvider2, true), + buildPaymentProviderApp(noPlayStoreUrlPaymentProvider, false) + ) + val mockedContext = createMockedContextAndSetDependencies(paymentProviderList, paymentProviderAppsList) + + //When + val paymentComponent = PaymentComponent(mockedContext, giniHealth!!) paymentComponent.loadPaymentProviderApps() // Then @@ -189,13 +294,13 @@ class PaymentComponentTest { fun `returns empty list when no payment providers installed`() = runTest { // Given val paymentProviderList = listOf( - notInstalledPaymentProvider + noPlayStoreUrlPaymentProvider ) coEvery { documentManager.getPaymentProviders() } returns Resource.Success(paymentProviderList) //When - val paymentComponent = PaymentComponent(context, giniHealth) + val paymentComponent = PaymentComponent(context!!, giniHealth!!) paymentComponent.loadPaymentProviderApps() // Then @@ -218,7 +323,7 @@ class PaymentComponentTest { coEvery { documentManager.getPaymentProviders() } returns Resource.Success(paymentProviderList) //When - val paymentComponent = PaymentComponent(context, giniHealth) + val paymentComponent = PaymentComponent(context!!, giniHealth!!) paymentComponent.loadPaymentProviderApps() // Then @@ -247,7 +352,7 @@ class PaymentComponentTest { coEvery { documentManager.getPaymentProviders() } returns Resource.Success(paymentProviderList) - val paymentComponent = PaymentComponent(context, giniHealth) + val paymentComponent = PaymentComponent(context!!, giniHealth!!) paymentComponent.loadPaymentProviderApps() paymentComponent.selectedPaymentProviderAppFlow.test { @@ -271,7 +376,7 @@ class PaymentComponentTest { fun `throws exception when trying to create ReviewFragment if no payment provider app is set`() = runTest { // Given val reviewConfiguration: ReviewConfiguration = mockk(relaxed = true) - val paymentComponent = PaymentComponent(context, giniHealth) + val paymentComponent = PaymentComponent(context!!, giniHealth!!) // When trying to instantiate fragment, then exception should be thrown paymentComponent.getPaymentReviewFragment("", reviewConfiguration) @@ -287,7 +392,7 @@ class PaymentComponentTest { // When every { paymentComponent.selectedPaymentProviderAppFlow } returns MutableStateFlow(SelectedPaymentProviderAppState.AppSelected(paymentProviderApp)) - //Then + // Then assertThat(paymentComponent.getPaymentReviewFragment("", reviewConfiguration)).isInstanceOf(ReviewFragment::class.java) } diff --git a/health-sdk/sdk/src/test/java/net/gini/android/health/sdk/paymentComponent/PaymentComponentViewTest.kt b/health-sdk/sdk/src/test/java/net/gini/android/health/sdk/paymentComponent/PaymentComponentViewTest.kt index a06051d6e6..1d4ba5fb9b 100644 --- a/health-sdk/sdk/src/test/java/net/gini/android/health/sdk/paymentComponent/PaymentComponentViewTest.kt +++ b/health-sdk/sdk/src/test/java/net/gini/android/health/sdk/paymentComponent/PaymentComponentViewTest.kt @@ -26,7 +26,6 @@ import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) class PaymentComponentViewTest { - private var activity : Activity? = null private var context: Context? = null private var scenario: ActivityScenario? = null private lateinit var paymentComponent: PaymentComponent @@ -34,13 +33,14 @@ class PaymentComponentViewTest { @Before fun setUp() { - activity = Robolectric.buildActivity(Activity::class.java).create().get() context = ApplicationProvider.getApplicationContext() context!!.setTheme(R.style.GiniHealthTheme) paymentComponent = PaymentComponent(context!!, mockk()) paymentComponentListener = mockk(relaxed = true) paymentComponent.listener = paymentComponentListener + // Needed to build the activity in which the custom component will be launched to be tested + Robolectric.buildActivity(Activity::class.java).create().get() scenario = ActivityScenario.launch( Intent(context, Activity::class.java) ) @@ -49,7 +49,6 @@ class PaymentComponentViewTest { @After fun tearDown() { - activity = null context = null scenario!!.close() }