Skip to content

Commit

Permalink
Push parameter encoding into BackendConfiguration.Call
Browse files Browse the repository at this point in the history
Now that we have a little more consistency throughout the library, here
we modify `BackendConfiguration.Call` (temporarily renamed to `Call2`
while we refactor) so that it can take parameter structs directly, then
encode them. This allows us to remove this common boilerplate from every
API call throughout the entire library:

``` go
if params != nil {
    commonParams = &params.Params
    body = &form.Values{}
    form.AppendTo(body, params)
}
```

Temporarily though, only the `charge/` package has been converted over
to show what it looks like before we convert everything.

There is a little bit of type trickiness that allows this to happen, and
which requires a somewhat advanced understanding of some Go language
semantics. We define a `ParamsContainer` interface as this:

``` go
type ParamsContainer interface {
    GetParams() *Params
}
```

Then on the `Params` struct itself, we define an implementation:

``` go
func (p *Params) GetParams() *Params {
    return p
}
```

In Go, whenever a struct is embedded in another struct, the host
inherits the function implementation of the struct which was embedded.
Because every parameter struct in the library embeds `Params`, they all
get a `GetParams` implementation and thus implement `ParamsContainer`
automatically:

``` go
type ChargeParams struct {
    Params `form:"*"`
    ...
}
```

This allows us to take a `ParamsContainer` from `Call`.

Most of this applies similarly for `ListParams`, and similarly `iter.go`
also gets a `Query2` and `GetIter2` to keep everything compiling while
we refactor everything. I did a little more refactoring in `iter.go` as
well just because there was quite a few very bad names in there. If this
patch lands, I'll probably refactor even more of it in a follow up.
  • Loading branch information
brandur committed Jun 9, 2018
1 parent a90cccc commit 228662e
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 75 deletions.
66 changes: 11 additions & 55 deletions charge/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,8 @@ func New(params *stripe.ChargeParams) (*stripe.Charge, error) {
}

func (c Client) New(params *stripe.ChargeParams) (*stripe.Charge, error) {
body := &form.Values{}
form.AppendTo(body, params)

charge := &stripe.Charge{}
err := c.B.Call("POST", "/charges", c.Key, body, &params.Params, charge)

err := c.B.Call2("POST", "/charges", c.Key, params, charge)
return charge, err
}

Expand All @@ -35,18 +31,9 @@ func Get(id string, params *stripe.ChargeParams) (*stripe.Charge, error) {
}

func (c Client) Get(id string, params *stripe.ChargeParams) (*stripe.Charge, error) {
var body *form.Values
var commonParams *stripe.Params

if params != nil {
commonParams = &params.Params
body = &form.Values{}
form.AppendTo(body, params)
}

path := stripe.FormatURLPath("/charges/%s", id)
charge := &stripe.Charge{}
err := c.B.Call("GET", stripe.FormatURLPath("/charges/%s", id), c.Key, body, commonParams, charge)

err := c.B.Call2("GET", path, c.Key, params, charge)
return charge, err
}

Expand All @@ -57,18 +44,9 @@ func Update(id string, params *stripe.ChargeParams) (*stripe.Charge, error) {
}

func (c Client) Update(id string, params *stripe.ChargeParams) (*stripe.Charge, error) {
var body *form.Values
var commonParams *stripe.Params

if params != nil {
commonParams = &params.Params
body = &form.Values{}
form.AppendTo(body, params)
}

path := stripe.FormatURLPath("/charges/%s", id)
charge := &stripe.Charge{}
err := c.B.Call("POST", stripe.FormatURLPath("/charges/%s", id), c.Key, body, commonParams, charge)

err := c.B.Call2("POST", path, c.Key, params, charge)
return charge, err
}

Expand All @@ -79,19 +57,9 @@ func Capture(id string, params *stripe.CaptureParams) (*stripe.Charge, error) {
}

func (c Client) Capture(id string, params *stripe.CaptureParams) (*stripe.Charge, error) {
var body *form.Values
var commonParams *stripe.Params

if params != nil {
commonParams = &params.Params
body = &form.Values{}
form.AppendTo(body, params)
}

path := stripe.FormatURLPath("/charges/%s/capture", id)
charge := &stripe.Charge{}

err := c.B.Call("POST", stripe.FormatURLPath("/charges/%s/capture", id), c.Key, body, commonParams, charge)

err := c.B.Call2("POST", path, c.Key, params, charge)
return charge, err
}

Expand All @@ -101,21 +69,10 @@ func List(params *stripe.ChargeListParams) *Iter {
return getC().List(params)
}

func (c Client) List(params *stripe.ChargeListParams) *Iter {
var body *form.Values
var lp *stripe.ListParams
var p *stripe.Params

if params != nil {
body = &form.Values{}
form.AppendTo(body, params)
lp = &params.ListParams
p = params.ToParams()
}

return &Iter{stripe.GetIter(lp, body, func(b *form.Values) ([]interface{}, stripe.ListMeta, error) {
func (c Client) List(listParams *stripe.ChargeListParams) *Iter {
return &Iter{stripe.GetIter2(listParams, func(p *stripe.Params, b *form.Values) ([]interface{}, stripe.ListMeta, error) {
list := &stripe.ChargeList{}
err := c.B.Call("GET", "/charges", c.Key, b, p, list)
err := c.B.CallRaw("GET", "/charges", c.Key, b, p, list)

ret := make([]interface{}, len(list.Data))
for i, v := range list.Data {
Expand Down Expand Up @@ -188,8 +145,7 @@ func CloseDispute(id string) (*stripe.Dispute, error) {

func (c Client) CloseDispute(id string) (*stripe.Dispute, error) {
dispute := &stripe.Dispute{}
err := c.B.Call("POST", stripe.FormatURLPath("/charges/%s/dispute/close", id), c.Key, nil, nil, dispute)

err := c.B.Call2("POST", stripe.FormatURLPath("/charges/%s/dispute/close", id), c.Key, nil, dispute)
return dispute, err
}

Expand Down
80 changes: 60 additions & 20 deletions iter.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import (
// Query is the function used to get a page listing.
type Query func(*form.Values) ([]interface{}, ListMeta, error)

// Query2 is the function used to get a page listing.
type Query2 func(*Params, *form.Values) ([]interface{}, ListMeta, error)

// Iter provides a convenient interface
// for iterating over the elements
// returned from paginated list API calls.
Expand All @@ -18,39 +21,76 @@ type Query func(*form.Values) ([]interface{}, ListMeta, error)
// Iterators are not thread-safe, so they should not be consumed
// across multiple goroutines.
type Iter struct {
cur interface{}
err error
meta ListMeta
params ListParams
qs *form.Values
query Query
values []interface{}
cur interface{}
err error
formValues *form.Values
listParams ListParams
meta ListMeta
query Query
query2 Query2
values []interface{}
}

// GetIter returns a new Iter for a given query and its options.
func GetIter(params *ListParams, qs *form.Values, query Query) *Iter {
func GetIter(listParams *ListParams, formValues *form.Values, query Query) *Iter {
iter := &Iter{}
iter.query = query

p := params
p := listParams
if p == nil {
p = &ListParams{}
}
iter.params = *p
iter.listParams = *p

q := qs
q := formValues
if q == nil {
q = &form.Values{}
}
iter.qs = q
iter.formValues = q

iter.getPage()
return iter
}

// TODO: After every list API call uses GetIter2, remove GetIter, then rename
// all instances of GetIter2 to GetIter. This only exists as a separate method
// to keep the build/tests working while we refactor.
func GetIter2(container ListParamsContainer, query Query2) *Iter {
var listParams *ListParams
formValues := &form.Values{}

if container != nil {
reflectValue := reflect.ValueOf(container)

// See the comment on Call in stripe.go.
if reflectValue.Kind() == reflect.Ptr && !reflectValue.IsNil() {
listParams = container.GetListParams()
form.AppendTo(formValues, container)
}
}

if listParams == nil {
listParams = &ListParams{}
}
iter := &Iter{
formValues: formValues,
listParams: *listParams,
query2: query,
}

iter.getPage()

return iter
}

func (it *Iter) getPage() {
it.values, it.meta, it.err = it.query(it.qs)
if it.params.EndingBefore != nil {
if it.query != nil {
it.values, it.meta, it.err = it.query(it.formValues)
} else {
it.values, it.meta, it.err = it.query2(it.listParams.GetParams(), it.formValues)
}

if it.listParams.EndingBefore != nil {
// We are moving backward,
// but items arrive in forward order.
reverse(it.values)
Expand All @@ -63,14 +103,14 @@ func (it *Iter) getPage() {
// It returns false when the iterator stops
// at the end of the list.
func (it *Iter) Next() bool {
if len(it.values) == 0 && it.meta.HasMore && !it.params.Single {
if len(it.values) == 0 && it.meta.HasMore && !it.listParams.Single {
// determine if we're moving forward or backwards in paging
if it.params.EndingBefore != nil {
it.params.EndingBefore = String(listItemID(it.cur))
it.qs.Set(EndingBefore, *it.params.EndingBefore)
if it.listParams.EndingBefore != nil {
it.listParams.EndingBefore = String(listItemID(it.cur))
it.formValues.Set(EndingBefore, *it.listParams.EndingBefore)
} else {
it.params.StartingAfter = String(listItemID(it.cur))
it.qs.Set(StartingAfter, *it.params.StartingAfter)
it.listParams.StartingAfter = String(listItemID(it.cur))
it.formValues.Set(StartingAfter, *it.listParams.StartingAfter)
}
it.getPage()
}
Expand Down
29 changes: 29 additions & 0 deletions params.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ const (
StartingAfter = "starting_after"
)

type ParamsContainer interface {
GetParams() *Params
}

type ListParamsContainer interface {
GetListParams() *ListParams
}

// Params is the structure that contains the common properties
// of any *Params structure.
type Params struct {
Expand Down Expand Up @@ -46,6 +54,13 @@ type Params struct {
StripeAccount *string `form:"-"` // Passed as header
}

// GetParams returns a Params struct (itself). It exists because any structs
// that embed Params will inherit it, and thus implement the ParamsContainer
// interface.
func (p *Params) GetParams() *Params {
return p
}

// ExtraValues are extra parameters that are attached to an API request.
// They're implemented as a custom type so that they can have their own
// AppendTo implementation.
Expand Down Expand Up @@ -200,6 +215,20 @@ func (p *ListParams) AddExpand(f string) {
p.Expand = append(p.Expand, &f)
}

// GetListParams returns a ListParams struct (itself). It exists because any
// structs that embed ListParams will inherit it, and thus implement the
// ListParamsContainer interface.
func (p *ListParams) GetListParams() *ListParams {
return p
}

// GetParams returns ListParams as a Params struct. It exists because any
// structs that embed Params will inherit it, and thus implement the
// ParamsContainer interface.
func (p *ListParams) GetParams() *Params {
return p.ToParams()
}

// SetStripeAccount sets a value for the Stripe-Account header.
func (p *ListParams) SetStripeAccount(val string) {
p.StripeAccount = &val
Expand Down
35 changes: 35 additions & 0 deletions stripe.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"net/url"
"os"
"os/exec"
"reflect"
"runtime"
"strings"
"sync"
Expand Down Expand Up @@ -72,6 +73,8 @@ func (a *AppInfo) formatUserAgent() string {
// This interface exists to enable mocking for during testing if needed.
type Backend interface {
Call(method, path, key string, body *form.Values, params *Params, v interface{}) error
Call2(method, path, key string, params ParamsContainer, v interface{}) error
CallRaw(method, path, key string, body *form.Values, params *Params, v interface{}) error
CallMultipart(method, path, key, boundary string, body io.Reader, params *Params, v interface{}) error
}

Expand Down Expand Up @@ -210,6 +213,38 @@ func SetBackend(backend SupportedBackend, b Backend) {

// Call is the Backend.Call implementation for invoking Stripe APIs.
func (s *BackendConfiguration) Call(method, path, key string, form *form.Values, params *Params, v interface{}) error {
return s.CallRaw(method, path, key, form, params, v)
}

// TODO: After every API call uses Call2, remove Call, then rename all
// instances of Call2 to Call. This only exists as a separate method to keep
// the build/tests working while we refactor.
func (s *BackendConfiguration) Call2(method, path, key string, params ParamsContainer, v interface{}) error {
var body *form.Values
var commonParams *Params

if params != nil {
// This is a little unfortunate, but Go makes it impossible to compare
// an interface value to nil without the use of the reflect package and
// its true disciples insist that this is a feature and not a bug.
//
// Here we do invoke reflect because (1) we have to reflect anyway to
// use encode with the form package, and (2) the corresponding removal
// of boilerplate that this enables makes the small performance penalty
// worth it.
reflectValue := reflect.ValueOf(params)

if reflectValue.Kind() == reflect.Ptr && !reflectValue.IsNil() {
commonParams = params.GetParams()
body = &form.Values{}
form.AppendTo(body, params)
}
}

return s.Call(method, path, key, body, commonParams, v)
}

func (s *BackendConfiguration) CallRaw(method, path, key string, form *form.Values, params *Params, v interface{}) error {
var body io.Reader
if form != nil && !form.Empty() {
data := form.Encode()
Expand Down

0 comments on commit 228662e

Please sign in to comment.