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

Eagerly finish activities if they're started with invalid args. #5883

Merged
merged 2 commits into from
Nov 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions payments-core/detekt-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@
<ID>ReturnCount:FraudDetectionDataJsonParser.kt$FraudDetectionDataJsonParser$override fun parse(json: JSONObject): FraudDetectionData?</ID>
<ID>ReturnCount:PaymentAuthWebViewClient.kt$PaymentAuthWebViewClient$private fun isReturnUrl(uri: Uri): Boolean</ID>
<ID>ReturnCount:WalletJsonParser.kt$WalletJsonParser$override fun parse(json: JSONObject): Wallet?</ID>
<ID>SwallowedException:ActivityUtils.kt$e: IllegalArgumentException</ID>
<ID>SwallowedException:PaymentUtils.kt$PaymentUtils$e: ClassCastException</ID>
<ID>SwallowedException:Stripe.kt$Stripe$exception: CardException</ID>
<ID>ThrowsCount:StripeApiRepository.kt$StripeApiRepository$@Throws( InvalidRequestException::class, AuthenticationException::class, CardException::class, APIException::class ) private fun handleApiError(response: StripeResponse&lt;String>)</ID>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.stripe.android.utils

import android.app.Activity

internal fun Activity.argsAreInvalid(argsProvider: () -> Unit): Boolean {
return try {
argsProvider()
false
} catch (e: IllegalArgumentException) {
finish()
true
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import com.stripe.android.Stripe
import com.stripe.android.databinding.AddPaymentMethodActivityBinding
import com.stripe.android.model.PaymentMethod
import com.stripe.android.model.PaymentMethodCreateParams
import com.stripe.android.utils.argsAreInvalid

/**
* Activity used to display a [AddPaymentMethodView] and receive the resulting
Expand Down Expand Up @@ -89,6 +90,9 @@ class AddPaymentMethodActivity : StripeActivity() {

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
if (argsAreInvalid { args }) {
return
}
configureView(args)
setResult(
Activity.RESULT_OK,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import com.stripe.android.R
import com.stripe.android.databinding.PaymentFlowActivityBinding
import com.stripe.android.model.ShippingInformation
import com.stripe.android.model.ShippingMethod
import com.stripe.android.utils.argsAreInvalid

/**
* Activity containing a two-part payment flow that allows users to provide a shipping address
Expand Down Expand Up @@ -61,6 +62,10 @@ class PaymentFlowActivity : StripeActivity() {
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)

if (argsAreInvalid { args }) {
return
}

val args = PaymentFlowActivityStarter.Args.create(intent)
args.windowFlags?.let { window.addFlags(it) }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import com.stripe.android.R
import com.stripe.android.core.exception.StripeException
import com.stripe.android.databinding.PaymentMethodsActivityBinding
import com.stripe.android.model.PaymentMethod
import com.stripe.android.utils.argsAreInvalid
import com.stripe.android.view.i18n.TranslatorManager

/**
Expand Down Expand Up @@ -77,6 +78,8 @@ class PaymentMethodsActivity : AppCompatActivity() {
)
}

private var earlyExitDueToIllegalState: Boolean = false

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
if (customerSession.isFailure) {
Expand All @@ -86,6 +89,10 @@ class PaymentMethodsActivity : AppCompatActivity() {
)
return
}
if (argsAreInvalid { args }) {
earlyExitDueToIllegalState = true
return
}

setContentView(viewBinding.root)

Expand Down Expand Up @@ -283,7 +290,9 @@ class PaymentMethodsActivity : AppCompatActivity() {
}

override fun onDestroy() {
viewModel.selectedPaymentMethodId = adapter.selectedPaymentMethod?.id
if (!earlyExitDueToIllegalState) {
viewModel.selectedPaymentMethodId = adapter.selectedPaymentMethod?.id
}
super.onDestroy()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ internal class ActivityScenarioFactory(
)
}

internal inline fun <reified T : Activity> create(): ActivityScenario<T> {
return ActivityScenario.launch(
Intent(context, T::class.java)
)
}

fun createAddPaymentMethodActivity() = create<AddPaymentMethodActivity>(
AddPaymentMethodActivityStarter.Args.Builder()
.setPaymentMethodType(PaymentMethod.Type.Card)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import android.widget.ProgressBar
import android.widget.TextView
import androidx.core.view.isGone
import androidx.core.view.isVisible
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.test.core.app.ApplicationProvider
Expand Down Expand Up @@ -69,6 +70,13 @@ class AddPaymentMethodActivityTest {
CustomerSession.instance = customerSession
}

@Test
fun testActivityIsFinishedWhenNoArgsPassed() {
activityScenarioFactory.create<AddPaymentMethodActivity>().use { activityScenario ->
assertThat(activityScenario.state).isEqualTo(Lifecycle.State.DESTROYED)
}
}

@Test
fun testConstructionForLocal() {
activityScenarioFactory.create<AddPaymentMethodActivity>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import android.content.Context
import androidx.arch.core.executor.testing.InstantTaskExecutorRule
import androidx.core.view.isGone
import androidx.core.view.isVisible
import androidx.lifecycle.Lifecycle
import androidx.test.core.app.ApplicationProvider
import com.google.common.truth.Truth.assertThat
import com.stripe.android.ApiKeyFixtures
Expand Down Expand Up @@ -49,6 +50,13 @@ class PaymentFlowActivityTest {
CustomerSession.initCustomerSession(context, ephemeralKeyProvider)
}

@Test
fun launchPaymentFlowActivity_withInvalidArgs_finishesActivity() {
activityScenarioFactory.create<PaymentFlowActivity>().use { activityScenario ->
assertThat(activityScenario.state).isEqualTo(Lifecycle.State.DESTROYED)
}
}

@Test
fun launchPaymentFlowActivity_withHideShippingInfoConfig_hidesShippingInfoView() {
activityScenarioFactory.create<PaymentFlowActivity>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package com.stripe.android.view
import android.app.Activity.RESULT_CANCELED
import android.content.Context
import android.view.View
import androidx.lifecycle.Lifecycle
import androidx.test.core.app.ApplicationProvider
import com.google.common.truth.Truth.assertThat
import com.stripe.android.ApiKeyFixtures
import com.stripe.android.CustomerSession
import com.stripe.android.PaymentConfiguration
Expand Down Expand Up @@ -44,6 +46,13 @@ class PaymentMethodsActivityTest {
PaymentConfiguration.init(context, ApiKeyFixtures.FAKE_PUBLISHABLE_KEY)
}

@Test
fun onCreate_finishesActivityWhenArgsAreMissing() {
activityScenarioFactory.create<PaymentMethodsActivity>().use { activityScenario ->
assertThat(activityScenario.state).isEqualTo(Lifecycle.State.DESTROYED)
}
}

@Test
fun onCreate_callsApiAndDisplaysProgressBarWhileWaiting() {
activityScenarioFactory.create<PaymentMethodsActivity>(
Expand Down