From 60860fbff092d8514d871d4c22120180fe404c7f Mon Sep 17 00:00:00 2001 From: Seth Friedman Date: Fri, 10 May 2024 01:04:38 -0400 Subject: [PATCH 1/4] Refactor voucher Authenticators into a single VoucherAuthenticator type Whenever we add a voucher LPM, we currently have to create a new Authenticator every time. However, we can instead make each piece of next action data conform to a common DisplayVoucherDetails interface and then delete all of the LPM-specific authenticators. --- .../com/stripe/android/model/StripeIntent.kt | 19 ++++---- .../authentication/BoletoAuthenticator.kt | 47 ------------------- .../authentication/MultibancoAuthenticator.kt | 47 ------------------- .../core/authentication/OxxoAuthenticator.kt | 47 ------------------- ...thenticator.kt => VoucherAuthenticator.kt} | 9 ++-- .../authentication/WebIntentAuthenticator.kt | 25 +--------- .../core/injection/AuthenticationModule.kt | 13 ++--- 7 files changed, 22 insertions(+), 185 deletions(-) delete mode 100644 payments-core/src/main/java/com/stripe/android/payments/core/authentication/BoletoAuthenticator.kt delete mode 100644 payments-core/src/main/java/com/stripe/android/payments/core/authentication/MultibancoAuthenticator.kt delete mode 100644 payments-core/src/main/java/com/stripe/android/payments/core/authentication/OxxoAuthenticator.kt rename payments-core/src/main/java/com/stripe/android/payments/core/authentication/{KonbiniAuthenticator.kt => VoucherAuthenticator.kt} (86%) diff --git a/payments-core/src/main/java/com/stripe/android/model/StripeIntent.kt b/payments-core/src/main/java/com/stripe/android/model/StripeIntent.kt index e84574f3e3a..faf08c833ba 100644 --- a/payments-core/src/main/java/com/stripe/android/model/StripeIntent.kt +++ b/payments-core/src/main/java/com/stripe/android/model/StripeIntent.kt @@ -172,6 +172,9 @@ sealed interface StripeIntent : StripeModel { } sealed class NextActionData : StripeModel { + interface DisplayVoucherDetails { + val hostedVoucherUrl: String? + } @Parcelize @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) @@ -189,8 +192,8 @@ sealed interface StripeIntent : StripeModel { /** * URL of a webpage containing the voucher for this OXXO payment. */ - val hostedVoucherUrl: String? = null - ) : NextActionData() + override val hostedVoucherUrl: String? = null + ) : NextActionData(), DisplayVoucherDetails @Parcelize @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) @@ -198,8 +201,8 @@ sealed interface StripeIntent : StripeModel { /** * URL of a webpage containing the voucher for this payment. */ - val hostedVoucherUrl: String? = null, - ) : NextActionData() + override val hostedVoucherUrl: String? = null, + ) : NextActionData(), DisplayVoucherDetails @Parcelize @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) @@ -207,8 +210,8 @@ sealed interface StripeIntent : StripeModel { /** * URL of a webpage containing the voucher for this payment. */ - val hostedVoucherUrl: String? = null, - ) : NextActionData() + override val hostedVoucherUrl: String? = null, + ) : NextActionData(), DisplayVoucherDetails @Parcelize @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) @@ -216,8 +219,8 @@ sealed interface StripeIntent : StripeModel { /** * URL of a webpage containing the voucher for this payment. */ - val hostedVoucherUrl: String? = null, - ) : NextActionData() + override val hostedVoucherUrl: String? = null, + ) : NextActionData(), DisplayVoucherDetails /** * Contains instructions for authenticating by redirecting your customer to another diff --git a/payments-core/src/main/java/com/stripe/android/payments/core/authentication/BoletoAuthenticator.kt b/payments-core/src/main/java/com/stripe/android/payments/core/authentication/BoletoAuthenticator.kt deleted file mode 100644 index 2effdaaebe2..00000000000 --- a/payments-core/src/main/java/com/stripe/android/payments/core/authentication/BoletoAuthenticator.kt +++ /dev/null @@ -1,47 +0,0 @@ -package com.stripe.android.payments.core.authentication - -import android.content.Context -import com.stripe.android.core.networking.ApiRequest -import com.stripe.android.model.StripeIntent -import com.stripe.android.model.StripeIntent.NextActionData -import com.stripe.android.payments.core.analytics.ErrorReporter -import com.stripe.android.view.AuthActivityStarterHost -import javax.inject.Inject -import javax.inject.Singleton - -/** - * [PaymentAuthenticator] for [NextActionData.DisplayBoletoDetails], redirects to - * [WebIntentAuthenticator] or [NoOpIntentAuthenticator] based on whether if there is a - * hostedVoucherUrl set. - */ -@Singleton -internal class BoletoAuthenticator @Inject constructor( - private val webIntentAuthenticator: WebIntentAuthenticator, - private val noOpIntentAuthenticator: NoOpIntentAuthenticator, - private val context: Context, -) : PaymentAuthenticator() { - override suspend fun performAuthentication( - host: AuthActivityStarterHost, - authenticatable: StripeIntent, - requestOptions: ApiRequest.Options - ) { - val detailsData = authenticatable.nextActionData as NextActionData.DisplayBoletoDetails - if (detailsData.hostedVoucherUrl == null) { - ErrorReporter.createFallbackInstance(context).report( - ErrorReporter.UnexpectedErrorEvent.MISSING_HOSTED_VOUCHER_URL, - additionalNonPiiParams = mapOf("lpm" to "boleto") - ) - noOpIntentAuthenticator.authenticate( - host, - authenticatable, - requestOptions - ) - } else { - webIntentAuthenticator.authenticate( - host, - authenticatable, - requestOptions - ) - } - } -} diff --git a/payments-core/src/main/java/com/stripe/android/payments/core/authentication/MultibancoAuthenticator.kt b/payments-core/src/main/java/com/stripe/android/payments/core/authentication/MultibancoAuthenticator.kt deleted file mode 100644 index 291de003f05..00000000000 --- a/payments-core/src/main/java/com/stripe/android/payments/core/authentication/MultibancoAuthenticator.kt +++ /dev/null @@ -1,47 +0,0 @@ -package com.stripe.android.payments.core.authentication - -import android.content.Context -import com.stripe.android.core.networking.ApiRequest -import com.stripe.android.model.StripeIntent -import com.stripe.android.model.StripeIntent.NextActionData -import com.stripe.android.payments.core.analytics.ErrorReporter -import com.stripe.android.view.AuthActivityStarterHost -import javax.inject.Inject -import javax.inject.Singleton - -/** - * [PaymentAuthenticator] for [NextActionData.DisplayMultibancoDetails], redirects to - * [WebIntentAuthenticator] or [NoOpIntentAuthenticator] based on whether if there is a - * hostedVoucherUrl set. - */ -@Singleton -internal class MultibancoAuthenticator @Inject constructor( - private val webIntentAuthenticator: WebIntentAuthenticator, - private val noOpIntentAuthenticator: NoOpIntentAuthenticator, - private val context: Context, -) : PaymentAuthenticator() { - override suspend fun performAuthentication( - host: AuthActivityStarterHost, - authenticatable: StripeIntent, - requestOptions: ApiRequest.Options - ) { - val detailsData = authenticatable.nextActionData as NextActionData.DisplayMultibancoDetails - if (detailsData.hostedVoucherUrl == null) { - ErrorReporter.createFallbackInstance(context).report( - ErrorReporter.UnexpectedErrorEvent.MISSING_HOSTED_VOUCHER_URL, - additionalNonPiiParams = mapOf("lpm" to "multibanco") - ) - noOpIntentAuthenticator.authenticate( - host, - authenticatable, - requestOptions - ) - } else { - webIntentAuthenticator.authenticate( - host, - authenticatable, - requestOptions - ) - } - } -} diff --git a/payments-core/src/main/java/com/stripe/android/payments/core/authentication/OxxoAuthenticator.kt b/payments-core/src/main/java/com/stripe/android/payments/core/authentication/OxxoAuthenticator.kt deleted file mode 100644 index 6d5cdcfa491..00000000000 --- a/payments-core/src/main/java/com/stripe/android/payments/core/authentication/OxxoAuthenticator.kt +++ /dev/null @@ -1,47 +0,0 @@ -package com.stripe.android.payments.core.authentication - -import android.content.Context -import com.stripe.android.core.networking.ApiRequest -import com.stripe.android.model.StripeIntent -import com.stripe.android.model.StripeIntent.NextActionData -import com.stripe.android.payments.core.analytics.ErrorReporter -import com.stripe.android.view.AuthActivityStarterHost -import javax.inject.Inject -import javax.inject.Singleton - -/** - * [PaymentAuthenticator] for [NextActionData.DisplayOxxoDetails], redirects to - * [WebIntentAuthenticator] or [NoOpIntentAuthenticator] based on whether if there is a - * hostedVoucherUrl set. - */ -@Singleton -internal class OxxoAuthenticator @Inject constructor( - private val webIntentAuthenticator: WebIntentAuthenticator, - private val noOpIntentAuthenticator: NoOpIntentAuthenticator, - private val context: Context, -) : PaymentAuthenticator() { - override suspend fun performAuthentication( - host: AuthActivityStarterHost, - authenticatable: StripeIntent, - requestOptions: ApiRequest.Options - ) { - val oxxoDetailsData = authenticatable.nextActionData as NextActionData.DisplayOxxoDetails - if (oxxoDetailsData.hostedVoucherUrl == null) { - ErrorReporter.createFallbackInstance(context).report( - ErrorReporter.UnexpectedErrorEvent.MISSING_HOSTED_VOUCHER_URL, - additionalNonPiiParams = mapOf("lpm" to "oxxo") - ) - noOpIntentAuthenticator.authenticate( - host, - authenticatable, - requestOptions - ) - } else { - webIntentAuthenticator.authenticate( - host, - authenticatable, - requestOptions - ) - } - } -} diff --git a/payments-core/src/main/java/com/stripe/android/payments/core/authentication/KonbiniAuthenticator.kt b/payments-core/src/main/java/com/stripe/android/payments/core/authentication/VoucherAuthenticator.kt similarity index 86% rename from payments-core/src/main/java/com/stripe/android/payments/core/authentication/KonbiniAuthenticator.kt rename to payments-core/src/main/java/com/stripe/android/payments/core/authentication/VoucherAuthenticator.kt index 52a55503d89..66b6f9fc444 100644 --- a/payments-core/src/main/java/com/stripe/android/payments/core/authentication/KonbiniAuthenticator.kt +++ b/payments-core/src/main/java/com/stripe/android/payments/core/authentication/VoucherAuthenticator.kt @@ -10,12 +10,12 @@ import javax.inject.Inject import javax.inject.Singleton /** - * [PaymentAuthenticator] for [NextActionData.DisplayKonbiniDetails], redirects to + * [PaymentAuthenticator] for [NextActionData.DisplayVoucherDetails], redirects to * [WebIntentAuthenticator] or [NoOpIntentAuthenticator] based on whether if there is a * hostedVoucherUrl set. */ @Singleton -internal class KonbiniAuthenticator @Inject constructor( +internal class VoucherAuthenticator @Inject constructor( private val webIntentAuthenticator: WebIntentAuthenticator, private val noOpIntentAuthenticator: NoOpIntentAuthenticator, private val context: Context, @@ -25,11 +25,10 @@ internal class KonbiniAuthenticator @Inject constructor( authenticatable: StripeIntent, requestOptions: ApiRequest.Options ) { - val detailsData = authenticatable.nextActionData as NextActionData.DisplayKonbiniDetails + val detailsData = authenticatable.nextActionData as NextActionData.DisplayVoucherDetails if (detailsData.hostedVoucherUrl == null) { ErrorReporter.createFallbackInstance(context).report( - ErrorReporter.UnexpectedErrorEvent.MISSING_HOSTED_VOUCHER_URL, - additionalNonPiiParams = mapOf("lpm" to "konbini") + ErrorReporter.UnexpectedErrorEvent.MISSING_HOSTED_VOUCHER_URL ) noOpIntentAuthenticator.authenticate( host, diff --git a/payments-core/src/main/java/com/stripe/android/payments/core/authentication/WebIntentAuthenticator.kt b/payments-core/src/main/java/com/stripe/android/payments/core/authentication/WebIntentAuthenticator.kt index d78739db757..7a5deff69b4 100644 --- a/payments-core/src/main/java/com/stripe/android/payments/core/authentication/WebIntentAuthenticator.kt +++ b/payments-core/src/main/java/com/stripe/android/payments/core/authentication/WebIntentAuthenticator.kt @@ -96,31 +96,10 @@ internal class WebIntentAuthenticator @Inject constructor( authUrl = nextActionData.webViewUrl.toString() returnUrl = nextActionData.returnUrl } - is StripeIntent.NextActionData.DisplayOxxoDetails -> { + is StripeIntent.NextActionData.DisplayVoucherDetails -> { // nextActionData.hostedVoucherUrl will never be null as AuthenticatorRegistry won't direct it here authUrl = nextActionData.hostedVoucherUrl.takeIf { it!!.isNotEmpty() } - ?: throw IllegalArgumentException("null hostedVoucherUrl for DisplayOxxoDetails") - returnUrl = null - shouldCancelIntentOnUserNavigation = false - } - is StripeIntent.NextActionData.DisplayBoletoDetails -> { - // nextActionData.hostedVoucherUrl will never be null as AuthenticatorRegistry won't direct it here - authUrl = nextActionData.hostedVoucherUrl.takeIf { it!!.isNotEmpty() } - ?: throw IllegalArgumentException("null hostedVoucherUrl for DisplayBoletoDetails") - returnUrl = null - shouldCancelIntentOnUserNavigation = false - } - is StripeIntent.NextActionData.DisplayKonbiniDetails -> { - // nextActionData.hostedVoucherUrl will never be null as AuthenticatorRegistry won't direct it here - authUrl = nextActionData.hostedVoucherUrl.takeIf { it!!.isNotEmpty() } - ?: throw IllegalArgumentException("null hostedVoucherUrl for DisplayKonbiniDetails") - returnUrl = null - shouldCancelIntentOnUserNavigation = false - } - is StripeIntent.NextActionData.DisplayMultibancoDetails -> { - // nextActionData.hostedVoucherUrl will never be null as AuthenticatorRegistry won't direct it here - authUrl = nextActionData.hostedVoucherUrl.takeIf { it!!.isNotEmpty() } - ?: throw IllegalArgumentException("null hostedVoucherUrl for DisplayMultibancoDetails") + ?: throw IllegalArgumentException("null hostedVoucherUrl for DisplayVoucherDetails") returnUrl = null shouldCancelIntentOnUserNavigation = false } diff --git a/payments-core/src/main/java/com/stripe/android/payments/core/injection/AuthenticationModule.kt b/payments-core/src/main/java/com/stripe/android/payments/core/injection/AuthenticationModule.kt index c82157e87a3..7f1667be30e 100644 --- a/payments-core/src/main/java/com/stripe/android/payments/core/injection/AuthenticationModule.kt +++ b/payments-core/src/main/java/com/stripe/android/payments/core/injection/AuthenticationModule.kt @@ -6,14 +6,11 @@ import com.stripe.android.PaymentRelayStarter import com.stripe.android.model.StripeIntent import com.stripe.android.model.StripeIntent.NextActionData import com.stripe.android.payments.DefaultReturnUrl -import com.stripe.android.payments.core.authentication.BoletoAuthenticator import com.stripe.android.payments.core.authentication.DefaultPaymentAuthenticatorRegistry -import com.stripe.android.payments.core.authentication.KonbiniAuthenticator -import com.stripe.android.payments.core.authentication.MultibancoAuthenticator -import com.stripe.android.payments.core.authentication.OxxoAuthenticator import com.stripe.android.payments.core.authentication.PaymentAuthenticator import com.stripe.android.payments.core.authentication.RealRedirectResolver import com.stripe.android.payments.core.authentication.RedirectResolver +import com.stripe.android.payments.core.authentication.VoucherAuthenticator import com.stripe.android.payments.core.authentication.WebIntentAuthenticator import com.stripe.android.view.AuthActivityStarterHost import dagger.Binds @@ -58,7 +55,7 @@ internal abstract class AuthenticationModule { @IntoMap @IntentAuthenticatorKey(NextActionData.DisplayMultibancoDetails::class) abstract fun bindsMultibancoAuthenticator( - multibancoAuthenticator: MultibancoAuthenticator + voucherAuthenticator: VoucherAuthenticator ): PaymentAuthenticator @IntentAuthenticatorMap @@ -66,7 +63,7 @@ internal abstract class AuthenticationModule { @IntoMap @IntentAuthenticatorKey(NextActionData.DisplayOxxoDetails::class) abstract fun bindsOxxoAuthenticator( - oxxoAuthenticator: OxxoAuthenticator + voucherAuthenticator: VoucherAuthenticator ): PaymentAuthenticator @IntentAuthenticatorMap @@ -74,7 +71,7 @@ internal abstract class AuthenticationModule { @IntoMap @IntentAuthenticatorKey(NextActionData.DisplayKonbiniDetails::class) abstract fun bindsKonbiniAuthenticator( - konbiniAuthenticator: KonbiniAuthenticator + voucherAuthenticator: VoucherAuthenticator ): PaymentAuthenticator @IntentAuthenticatorMap @@ -82,7 +79,7 @@ internal abstract class AuthenticationModule { @IntoMap @IntentAuthenticatorKey(NextActionData.DisplayBoletoDetails::class) abstract fun bindsBoletoAuthenticator( - boletoAuthenticator: BoletoAuthenticator + voucherAuthenticator: VoucherAuthenticator ): PaymentAuthenticator @IntentAuthenticatorMap From 85e89d0785c486902960bb18fe43d5d046502f08 Mon Sep 17 00:00:00 2001 From: Seth Friedman Date: Fri, 10 May 2024 17:01:51 -0400 Subject: [PATCH 2/4] Add back type info for errors --- .../payments/core/authentication/VoucherAuthenticator.kt | 4 +++- .../payments/core/authentication/WebIntentAuthenticator.kt | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/payments-core/src/main/java/com/stripe/android/payments/core/authentication/VoucherAuthenticator.kt b/payments-core/src/main/java/com/stripe/android/payments/core/authentication/VoucherAuthenticator.kt index 66b6f9fc444..98c84267e50 100644 --- a/payments-core/src/main/java/com/stripe/android/payments/core/authentication/VoucherAuthenticator.kt +++ b/payments-core/src/main/java/com/stripe/android/payments/core/authentication/VoucherAuthenticator.kt @@ -26,9 +26,11 @@ internal class VoucherAuthenticator @Inject constructor( requestOptions: ApiRequest.Options ) { val detailsData = authenticatable.nextActionData as NextActionData.DisplayVoucherDetails + println(authenticatable.nextActionType) if (detailsData.hostedVoucherUrl == null) { ErrorReporter.createFallbackInstance(context).report( - ErrorReporter.UnexpectedErrorEvent.MISSING_HOSTED_VOUCHER_URL + ErrorReporter.UnexpectedErrorEvent.MISSING_HOSTED_VOUCHER_URL, + additionalNonPiiParams = mapOf("next_action_type" to (authenticatable.nextActionType?.code ?: "")) ) noOpIntentAuthenticator.authenticate( host, diff --git a/payments-core/src/main/java/com/stripe/android/payments/core/authentication/WebIntentAuthenticator.kt b/payments-core/src/main/java/com/stripe/android/payments/core/authentication/WebIntentAuthenticator.kt index 7a5deff69b4..c3c77b3208d 100644 --- a/payments-core/src/main/java/com/stripe/android/payments/core/authentication/WebIntentAuthenticator.kt +++ b/payments-core/src/main/java/com/stripe/android/payments/core/authentication/WebIntentAuthenticator.kt @@ -99,7 +99,9 @@ internal class WebIntentAuthenticator @Inject constructor( is StripeIntent.NextActionData.DisplayVoucherDetails -> { // nextActionData.hostedVoucherUrl will never be null as AuthenticatorRegistry won't direct it here authUrl = nextActionData.hostedVoucherUrl.takeIf { it!!.isNotEmpty() } - ?: throw IllegalArgumentException("null hostedVoucherUrl for DisplayVoucherDetails") + ?: throw IllegalArgumentException( + "null hostedVoucherUrl for ${authenticatable.nextActionType?.code}" + ) returnUrl = null shouldCancelIntentOnUserNavigation = false } From 8fa2f8a14658fce669fb17b303e745ef3010e244 Mon Sep 17 00:00:00 2001 From: Seth Friedman Date: Fri, 10 May 2024 17:14:25 -0400 Subject: [PATCH 3/4] Remove stray println --- .../android/payments/core/authentication/VoucherAuthenticator.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/payments-core/src/main/java/com/stripe/android/payments/core/authentication/VoucherAuthenticator.kt b/payments-core/src/main/java/com/stripe/android/payments/core/authentication/VoucherAuthenticator.kt index 98c84267e50..711429461aa 100644 --- a/payments-core/src/main/java/com/stripe/android/payments/core/authentication/VoucherAuthenticator.kt +++ b/payments-core/src/main/java/com/stripe/android/payments/core/authentication/VoucherAuthenticator.kt @@ -26,7 +26,6 @@ internal class VoucherAuthenticator @Inject constructor( requestOptions: ApiRequest.Options ) { val detailsData = authenticatable.nextActionData as NextActionData.DisplayVoucherDetails - println(authenticatable.nextActionType) if (detailsData.hostedVoucherUrl == null) { ErrorReporter.createFallbackInstance(context).report( ErrorReporter.UnexpectedErrorEvent.MISSING_HOSTED_VOUCHER_URL, From ce4219a19f74d0bd8a0c5fe819674b55b7e11de1 Mon Sep 17 00:00:00 2001 From: Seth Friedman Date: Fri, 10 May 2024 17:39:28 -0400 Subject: [PATCH 4/4] Restrict DisplayVoucherDetails to library group --- .../src/main/java/com/stripe/android/model/StripeIntent.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/payments-core/src/main/java/com/stripe/android/model/StripeIntent.kt b/payments-core/src/main/java/com/stripe/android/model/StripeIntent.kt index faf08c833ba..ed62b1cbdeb 100644 --- a/payments-core/src/main/java/com/stripe/android/model/StripeIntent.kt +++ b/payments-core/src/main/java/com/stripe/android/model/StripeIntent.kt @@ -172,6 +172,7 @@ sealed interface StripeIntent : StripeModel { } sealed class NextActionData : StripeModel { + @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) interface DisplayVoucherDetails { val hostedVoucherUrl: String? }