Remove ClientTokenProvider #831
Conversation
| }); | ||
| } else { | ||
| braintreeClient.getConfiguration((configuration, e) -> { | ||
| if (configuration == null) { | ||
| callback.onResult(null, null); |
There was a problem hiding this comment.
Unrelated, but this is like the only callback with two success parameters and no error parameter. We should be able to fix that for v5 since we can make breaking changes.
| @@ -69,111 +74,26 @@ open class BraintreeClient @VisibleForTesting internal constructor( | |||
| */ | |||
| @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) | |||
| constructor(clientParams: ClientParams) : | |||
There was a problem hiding this comment.
The effect of this change for merchants is that if they pass an invalid authentication string to their client constructors, it will be returned as a config error (error fetching configuration, rather than error fetching authorization).
On iOS, the BTAPIClient (closest to android's BraintreeClient) has a failable init which gets triggered if some of the SDKs manual checks on the clientToken after base64 decoding fail.
We could move up that logic in the Android sequence of events to more closely mirror iOS and return an error sooner if the auth is invalid (I think it lives here). Total take it or leave it comment - curious your thoughts
There was a problem hiding this comment.
The concept of a failable init doesn't really exist in Kotlin or Java - there are some hacky ways to achieve that behavior but I don't think it would be an expected integration pattern for merchants. We could always throw an error in the init method, but that means merchants have to handle the error which I think is less ideal than calling back an error further in the flow. To make it more explicit, we could add a check to the four BraintreeClient methods that fail for auth (http calls and getConfiguration) if auth is InvalidAuthorization and callback an authorization error at the top of those methods so that the errors are more clear. Requires some code duplication but I think that's probably the best merchant experience - what do you think?
There was a problem hiding this comment.
@scannillo I decided to go ahead and callback an auth error in the four BraintreeClient methods that would error on config fetch for invalid auth. In v4 we had a helpful auth error message directing merchants to docs so I think that's a good message to keep.
Summary of changes
ClientTokenProviderand asynchronous loading of authorization in favor of having merchants instantiate clients with authorization directly (see Instantiate BraintreeClient Within Clients #830).BraintreeClientconstructor and addauthorizationparameterAuthorizationLoaderclassBraintreeClientinternal and removing public constructors also has the side effect of makinggetConfigurationprivate. As far as I can tell, getConfiguration is not publicly available in iOS, so I think this is an ok change.Note: Update base to
v5after #830 is merged.Checklist
Authors