Use STPAPIClient.shared by default #1469
Conversation
…new one. This means its api client has the same configuration as the shared one. This gets us closer to "By default, the Stripe SDK uses the STPAPIClient.shared to make API calls."
yuki-stripe
force-pushed
the
yuki/customercontext-use-shared-apiclient
branch
from
265278f
to
5f33a47
Compare
…ent to APIRequest
…nt.sharerd insted of APIClient.clientWithEphemeralKey
|
@csabol-stripe Ready for another pass. I discovered 2 more classes that make API requests, both using ephemeral keys. I think it makes sense that they use sharedClient too. Manually tested Basic Integration again. |
…STPAPIClient shared], except for CustomerContext, which requires more refactoring since it uses APIClient class methods.
…rty on CustomerContext
yuki-stripe
changed the title
|
Expanded scope of PR after discussion, see revised summary. |
| @@ -91,7 +90,7 @@ - (void)commonInitWithConfiguration:(STPPaymentConfiguration *)configuration { | |||
| _configuration = configuration; | |||
| _shippingAddress = nil; | |||
| _hasUsedShippingAddress = NO; | |||
| _apiClient = [[STPAPIClient alloc] initWithConfiguration:configuration]; | |||
| _apiClient = [STPAPIClient sharedClient]; | |||
There was a problem hiding this comment.
hmm, these changes are the most concerning to me. If stripeAccount (and other STPAPIClient configurations) is still a property on STPPaymentConfiguration this should probably respect those. Is the plan to deprecate on STPPaymentConfiguration?
There was a problem hiding this comment.
Yes, plan is to kill PaymentConfiguration.{stripeAccount, publishableKey} in a follow-up PR
There was a problem hiding this comment.
mind putting up that patch in parallel with this one so I can review both together?
…ugh they don't make network requests themselves, they initialize others that do.
Here's a graph:
PaymentContext
- PaymentOptionsVC
- PaymentOptionsInternalVC
- AddCardVC
- BankSelectionVC
- AddCardVC
APIClient configuration needs to flow from every node to leaf.
[23] Project files and iOS 13 changes
Summary
Make
STPPaymentContext,STPCustomerContext,STPPinManagementService,STPPushProvisioningContext,STPAddCardViewController:STPAPIClient.sharedPreviously, they used various APIClient instances, leading to inconsistent and unclear configurations of APIClient properties like
stripeAccount.Motivation
https://paper.dropbox.com/doc/psyduck-why-iOS-SDK-API-Configuration-LTvIMxj6CBy1Mw4ntaU0l#:uid=436909486618175148450143&h2=Proposal
https://jira.corp.stripe.com/browse/IOS-1437
Testing
Manually test basic integration app and make sure customer things work.