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

(Beta) simplify stripe.Backend interface #1778

Closed
wants to merge 4 commits into from

Conversation

richardm-stripe
Copy link
Contributor

@richardm-stripe richardm-stripe commented Dec 5, 2023

Along similar lines to stripe/stripe-java#1702 in stripe-java.

Summary

Introduces stripe.StripeRequest struct, and changes Backend interface:

 type Backend interface {
- 	Call(method, path, key string, params ParamsContainer, v LastResponseSetter) error
+  	Call(req StripeRequest, v LastResponseSetter) error
- 	CallStreaming(method, path, key string, params ParamsContainer, v StreamingLastResponseSetter) error
+ 	CallStreaming(req StripeRequest, v StreamingLastResponseSetter) error
- 	CallRaw(method, path, key string, body *form.Values, params *Params, v LastResponseSetter) error
- 	CallMultipart(method, path, key, boundary string, body *bytes.Buffer, params *Params, v LastResponseSetter) error
- 	SetMaxNetworkRetries(maxNetworkRetries int64)
 }

Setting up StripeRequest is broken up into two steps: To achieve the equivalent of b.Call(method, path, key, params, &result), you now do

sr := stripe.NewStripeRequest(method, path, key)
sr.SetParams(params)
b.Call(sr, &result)

To achieve the equivalent of b.CallRaw(method, path, key, body, params, &result):

sr := stripe.NewStripeRequest(method, path, key)
sr.SetRawForm(params, body)
b.Call(sr, &result)

To achieve the equivalent of b.CallMultipart(method, path, key, boundary, body, params, &result):

sr := stripe.NewStripeRequest(method, path, key)
sr.SetMultipart(params, boundary, body)
b.Call(sr, &result)

Why

This is breaking, but this:

  • will allow us to pass new, optional data to.Call by adding it onto StripeRequest. Otherwise, we would have to break the interface and add a new parameter to .Call or a new method to Backend, or we would have to smuggle the data onto Params, which isn't really an appropriate place.
    • For instance: usage that we are soon adding to all libraries,
    • or ApiMode that is used by RawRequest to send preview requests.
  • it decreases the surface area necessary to implement Backend, which should make the experience better for future users who need to write an implementation of Backend (for mocking in their tests?) -- Call is essentially a convenience wrapper for CallRaw, but by putting both on the interface, we force users to implement both.

Planned follow-ups

  • I should be able to remove RawRequestBackend now and subsume it into Backend. (Will do in a follow-up, to keep the diff manageable).
  • Usage

@richardm-stripe richardm-stripe changed the title (Beta) [WIP] simplify stripe.Backend interface (Beta) simplify stripe.Backend interface Dec 6, 2023
@richardm-stripe richardm-stripe marked this pull request as ready for review December 6, 2023 00:09
@richardm-stripe richardm-stripe requested review from a team and anniel-stripe and removed request for a team December 6, 2023 00:10
@richardm-stripe
Copy link
Contributor Author

Closing: we're going to resurrect this when there are other reasons to make infrastructure changes. For now, we're going to avoid the breaking change and smuggle any additional parameters onto Params.

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.

1 participant