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.

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

Fixes ApiRequestorAdapter requests with BaseAddress other than Api #3007

Merged
merged 2 commits into from
Oct 17, 2024

Conversation

jar-stripe
Copy link
Contributor

@jar-stripe jar-stripe commented Oct 17, 2024

Why?

v46 of the SDK introduced a service accessors into StripeClient and a internal ApiRequestor classes for implementing the actual request issuing and response parsing logic used by the service classes. The ApiRequestorAdapter class is used to support older-style SDK usage patterns, e.g.

var svc = new CustomerService();
var customer = svc.Get("cust_...");

and also to support any custom implementations of IStripeClient. When a service is created directly like this, Stripe.net wraps the StripeConfiguration.StripeClient client object with this adapter to adapt the client to an ApiRequestor.

A user reported a problem with using the OAuthTokenService service with the pattern described above (#3006) where the SDK was not using the correct API base address. The root issue appears to be that BaseAddress specified by the OAuthTokenService.Create method is getting replaced with the API base address when it passes through the ApiRequestorAdapter, because the IStripeClient RequestAsync method does not accept a BaseAddress argument. To fix this would introduce a breaking change in IStripeClient; this PR implements a backwards compatible fix by utilizing the RequestOption.BaseUrl property.

What?

  • changes ApiRequestorAdapter to set the RequestOptions.BaseUrl if the baseAddress argument is anything other than API
  • change sLiveApiRequestor MakeStripeRequest to use the BaseUrl to override baseAddress, if BaseUrl is specified
  • adds unit tests for ApiRequestorAdapter RequestAsync with different base addresses
  • adds unit tests for OAuthTokenService Create and CreateAsync using the older-style instantiation pattern

Changelog

  • fixes bug where OAuthTokenService created without an explicit StripeClient accesses the wrong base url

…ss is discarded

changed Service.RequestAsync to utilize the RequestOptions BaseUrl if it is present

changed new ApiRequestorAdapter to ApiRequestorAdapter.Adapt

added "legacy service usage" tests to OAuthTokenServiceTest to confirm non-ApiBase service calls made this way still work
Copy link
Member

@xavdid-stripe xavdid-stripe left a comment

Choose a reason for hiding this comment

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

Looks good! Should also add a ## Changlog in the PR description so it shows up in release notes

@jar-stripe jar-stripe merged commit 2fc2a0d into master Oct 17, 2024
4 checks passed
@jar-stripe jar-stripe deleted the jar/api-requestor-adapter-base-address branch October 17, 2024 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants