-
Notifications
You must be signed in to change notification settings - Fork 662
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
Add CustomerSessionClientSecret
validation & remodel ElementsSessionManager
response.
#9373
Add CustomerSessionClientSecret
validation & remodel ElementsSessionManager
response.
#9373
Conversation
40f506d
to
49f3cc2
Compare
…onManager` response.
49f3cc2
to
2cc22be
Compare
}.getOrThrow() | ||
} | ||
} | ||
} | ||
|
||
private fun validateCustomerSessionClientSecret(customerSessionClientSecret: String) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a way we can share this code with payment sheet rather than re-implementing it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a CustomerSessionClientSecretProvider
that does the actual validation. Error messages are still different between both implementations given the context in how the errors are produced is different between both PaymentSheet
and CustomerSheet
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great, thanks!
private val timeProvider: () -> Long, | ||
@IOContext private val workContext: CoroutineContext, | ||
) : CustomerSessionElementsSessionManager { | ||
@Volatile | ||
private var cachedCustomerEphemeralKey: CachedCustomerEphemeralKey = CachedCustomerEphemeralKey.None | ||
private var cachedCustomerEphemeralKey: CachedCustomerEphemeralKey? = null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decided to convert this back to a nullable type with no sealed interface since None
serves no value after changes made here.
Diffuse output:
APK
DEX
|
e197a9c
to
525111a
Compare
|
||
suspend fun fetchElementsSession(): Result<ElementsSession> | ||
suspend fun fetchElementsSession(): Result<CustomerSessionElementsSession> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fwiw I think that CustomerSessionElementsSession
could also be a completely internal type, and the return type of this function could be unchanged. I don't think there are any consumers of this function yet though, so it's unclear to me which will be more ergonomic!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually CustomerSessionPaymentMethodDataSource
consumes the result of this function! The type
helps ensure ElementsSession.Customer
is not null unlike the ElementsSession
type which can have it null. This way, CustomerSessionElementsSessionManager
is responsible for ensuring that the customer
field is available and not the individual data sources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh gotcha. Nice! That's great
}.getOrThrow() | ||
} | ||
} | ||
} | ||
|
||
private fun validateCustomerSessionClientSecret(customerSessionClientSecret: String) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great, thanks!
Summary
Add
CustomerSessionClientSecret
validation & remodelElementsSessionManager
response.Motivation
We validate on the client-side if the provided key is a valid customer session key for
PaymentSheet
and should do the same forCustomerSheet
. With this validation, we can now expectelements/session
to return acustomer
field soElementsSessionManager
now provides a response withElementsSession
and a non-null customer.Testing