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.

Sign up for GitHub

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

Jump to bottom

Refactor voucher Authenticators into a single VoucherAuthenticator type #8448

Merged
merged 4 commits into from
May 13, 2024
Merged

Refactor voucher Authenticators into a single VoucherAuthenticator type #8448

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
60860fb
Refactor voucher Authenticators into a single VoucherAuthenticator type
sfriedman-stripe May 10, 2024
85e89d0
Add back type info for errors
sfriedman-stripe May 10, 2024
8fa2f8a
Remove stray println
sfriedman-stripe May 10, 2024
ce4219a
Restrict DisplayVoucherDetails to library group
sfriedman-stripe May 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 11 additions & 8 deletions payments-core/src/main/java/com/stripe/android/model/StripeIntent.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,9 @@ sealed interface StripeIntent : StripeModel {
}

sealed class NextActionData : StripeModel {
interface DisplayVoucherDetails {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could also add another val paymentMethodType: PaymentMethod.Type here, and manually add it to each of the deserialized types. That would allow you to continue to have the data in the error reporter.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a little weird though for DisplayOxxoDetails to need to have a payment method property to know that it's related to Oxxo though, right? It's right there in the name. Maybe there's something I should be doing with reflection here

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's a little weird, I'd think about the type (DisplayOxxoDetails) telling the interface (DisplayVoucherDetails) some metadata about the implementation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Took a stab at using the NextActionType on the authenticatable instead. Lmk what you think!

val hostedVoucherUrl: String?
}

@Parcelize
@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
Expand All @@ -189,35 +192,35 @@ 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)
data class DisplayBoletoDetails(
/**
* 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)
data class DisplayKonbiniDetails(
/**
* 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)
data class DisplayMultibancoDetails(
/**
* 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
Expand Down
47 changes: 0 additions & 47 deletions ...core/src/main/java/com/stripe/android/payments/core/authentication/BoletoAuthenticator.kt
View file
Open in desktop

This file was deleted.

47 changes: 0 additions & 47 deletions .../src/main/java/com/stripe/android/payments/core/authentication/MultibancoAuthenticator.kt
View file
Open in desktop

This file was deleted.

47 changes: 0 additions & 47 deletions ...s-core/src/main/java/com/stripe/android/payments/core/authentication/OxxoAuthenticator.kt
View file
Open in desktop

This file was deleted.

9 changes: 5 additions & 4 deletions ...re/authentication/KonbiniAuthenticator.kt → ...re/authentication/VoucherAuthenticator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -25,11 +25,12 @@ internal class KonbiniAuthenticator @Inject constructor(
authenticatable: StripeIntent,
requestOptions: ApiRequest.Options
) {
val detailsData = authenticatable.nextActionData as NextActionData.DisplayKonbiniDetails
val detailsData = authenticatable.nextActionData as NextActionData.DisplayVoucherDetails
println(authenticatable.nextActionType)
if (detailsData.hostedVoucherUrl == null) {
ErrorReporter.createFallbackInstance(context).report(
ErrorReporter.UnexpectedErrorEvent.MISSING_HOSTED_VOUCHER_URL,
additionalNonPiiParams = mapOf("lpm" to "konbini")
sfriedman-stripe marked this conversation as resolved.
Show resolved Hide resolved
additionalNonPiiParams = mapOf("next_action_type" to (authenticatable.nextActionType?.code ?: ""))
)
noOpIntentAuthenticator.authenticate(
host,
Expand Down
27 changes: 4 additions & 23 deletions ...e/src/main/java/com/stripe/android/payments/core/authentication/WebIntentAuthenticator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,31 +96,12 @@ 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 ${authenticatable.nextActionType?.code}"
)
returnUrl = null
shouldCancelIntentOnUserNavigation = false
}
Expand Down
13 changes: 5 additions & 8 deletions ...nts-core/src/main/java/com/stripe/android/payments/core/injection/AuthenticationModule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -58,31 +55,31 @@ internal abstract class AuthenticationModule {
@IntoMap
@IntentAuthenticatorKey(NextActionData.DisplayMultibancoDetails::class)
abstract fun bindsMultibancoAuthenticator(
multibancoAuthenticator: MultibancoAuthenticator
voucherAuthenticator: VoucherAuthenticator
): PaymentAuthenticator<StripeIntent>

@IntentAuthenticatorMap
@Binds
@IntoMap
@IntentAuthenticatorKey(NextActionData.DisplayOxxoDetails::class)
abstract fun bindsOxxoAuthenticator(
oxxoAuthenticator: OxxoAuthenticator
voucherAuthenticator: VoucherAuthenticator
): PaymentAuthenticator<StripeIntent>

@IntentAuthenticatorMap
@Binds
@IntoMap
@IntentAuthenticatorKey(NextActionData.DisplayKonbiniDetails::class)
abstract fun bindsKonbiniAuthenticator(
konbiniAuthenticator: KonbiniAuthenticator
voucherAuthenticator: VoucherAuthenticator
): PaymentAuthenticator<StripeIntent>

@IntentAuthenticatorMap
@Binds
@IntoMap
@IntentAuthenticatorKey(NextActionData.DisplayBoletoDetails::class)
abstract fun bindsBoletoAuthenticator(
boletoAuthenticator: BoletoAuthenticator
voucherAuthenticator: VoucherAuthenticator
): PaymentAuthenticator<StripeIntent>

@IntentAuthenticatorMap
Expand Down