Skip to content

feat: Introduce Context #287

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions client/base_client.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package client

import (
"context"
"net/http"
"net/url"
"time"
Expand All @@ -13,4 +14,6 @@ type BaseClient interface {
headers map[string]interface{}, body ...byte) (*http.Response, error)
SetOauth(auth OAuth)
OAuth() OAuth
SendRequestWithContext(ctx context.Context, method string, rawURL string, data url.Values,
headers map[string]interface{}, body ...byte) (*http.Response, error)
}
94 changes: 93 additions & 1 deletion client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (

var alphanumericRegex *regexp.Regexp
var delimitingRegex *regexp.Regexp
var goVersion string
var goVersionOnce sync.Once

func init() {
alphanumericRegex = regexp.MustCompile(`^[a-zA-Z0-9]*$`)
Expand Down Expand Up @@ -148,7 +150,7 @@ func (c *Client) SendRequest(method string, rawURL string, data url.Values,
}

valueReader := &strings.Reader{}
goVersion := runtime.Version()
goVersion = getGoVersion()
var req *http.Request

//For HTTP GET Method there are no body parameters. All other parameters like query, path etc
Expand Down Expand Up @@ -220,6 +222,89 @@ func (c *Client) SendRequest(method string, rawURL string, data url.Values,
return c.doWithErr(req)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

OK - I still think we should modify SendRequest to have no method body and just instead call

return SendRequestWithContext(context.Background(), method, rawURL, data, headers, body)

func (c *Client) SendRequestWithContext(ctx context.Context, method string, rawURL string, data url.Values,
headers map[string]interface{}, body ...byte) (*http.Response, error) {

contentType := extractContentTypeHeader(headers)

u, err := url.Parse(rawURL)
if err != nil {
return nil, err
}

valueReader := &strings.Reader{}
goVersion := runtime.Version()
var req *http.Request

//For HTTP GET Method there are no body parameters. All other parameters like query, path etc
// are added as information in the url itself. Also while Content-Type is json, we are sending
// json body. In that case, data variable contains all other parameters than body, which is the
//same case as GET method. In that case as well all parameters will be added to url
if method == http.MethodGet || method == http.MethodDelete || contentType == jsonContentType {
if data != nil {
v, _ := form.EncodeToStringWith(data, delimiter, escapee, keepZeros)
s := delimitingRegex.ReplaceAllString(v, "")

u.RawQuery = s
}
}

//data is already processed and information will be added to u(the url) in the
//previous step. Now body will solely contain json payload
if contentType == jsonContentType {
req, err = http.NewRequestWithContext(ctx, method, u.String(), bytes.NewBuffer(body))
if err != nil {
return nil, err
}
} else {
// Here the HTTP POST methods which do not have json content type are processed
// All the values will be added in data and encoded (all body, query, path parameters)
if method == http.MethodPost || method == http.MethodPut || method == http.MethodPatch {
valueReader = strings.NewReader(data.Encode())
}
req, err = http.NewRequestWithContext(context.Background(), method, u.String(), valueReader)
if err != nil {
return nil, err
}

}

credErr := c.validateCredentials()
if credErr != nil {
return nil, credErr
}
if c.OAuth() == nil && c.Username != "" && c.Password != "" {
req.SetBasicAuth(c.basicAuth())
}

// E.g. "User-Agent": "twilio-go/1.0.0 (darwin amd64) go/go1.17.8"
userAgentOnce.Do(func() {
baseUserAgent = fmt.Sprintf("twilio-go/%s (%s %s) go/%s", LibraryVersion, runtime.GOOS, runtime.GOARCH, goVersion)
})
userAgent := baseUserAgent

if len(c.UserAgentExtensions) > 0 {
userAgent += " " + strings.Join(c.UserAgentExtensions, " ")
}
if c.OAuth() != nil {
oauth := c.OAuth()
token, _ := c.OAuth().GetAccessToken(context.TODO())
Copy link
Contributor

Choose a reason for hiding this comment

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

Should pass through the context here as well.

if token != "" {
req.Header.Add("Authorization", "Bearer "+token)
}
c.SetOauth(oauth) // Set the OAuth token in the client which gets nullified after the token fetch
} else if c.Username != "" && c.Password != "" {
req.SetBasicAuth(c.basicAuth())
}

req.Header.Add("User-Agent", userAgent)

for k, v := range headers {
req.Header.Add(k, fmt.Sprint(v))
}
return c.doWithErr(req)
}

// SetAccountSid sets the Client's accountSid field
func (c *Client) SetAccountSid(sid string) {
c.accountSid = sid
Expand All @@ -237,3 +322,10 @@ func (c *Client) SetOauth(oauth OAuth) {
func (c *Client) OAuth() OAuth {
return c.oAuth
}

func getGoVersion() string {
goVersionOnce.Do(func() {
goVersion = runtime.Version()
})
return goVersion
}
5 changes: 3 additions & 2 deletions client/page_util.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package client

import (
"context"
"encoding/json"
"fmt"
"strings"
Expand All @@ -25,13 +26,13 @@ func ReadLimits(pageSize *int, limit *int) int {
}
}

func GetNext(baseUrl string, response interface{}, getNextPage func(nextPageUri string) (interface{}, error)) (interface{}, error) {
func GetNextWithContext(ctx context.Context, baseUrl string, response interface{}, getNextPage func(ctx context.Context, nextPageUri string) (interface{}, error)) (interface{}, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to add this as a second method, similar to the SendRequestWithContext, otherwise we're going to break everyone's clients.

nextPageUrl, err := getNextPageUrl(baseUrl, response)
if err != nil {
return nil, err
}

return getNextPage(nextPageUrl)
return getNextPage(ctx, nextPageUrl)
}

func toMap(s interface{}) (map[string]interface{}, error) {
Expand Down
7 changes: 4 additions & 3 deletions client/page_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package client

import (
"bytes"
"context"
"encoding/json"
"io"
"net/http"
Expand Down Expand Up @@ -140,7 +141,7 @@ type testMessage struct {
To *string `json:"to,omitempty"`
}

func getSomething(nextPageUrl string) (interface{}, error) {
func getSomething(ctx context.Context, nextPageUrl string) (interface{}, error) {
return nextPageUrl, nil
}

Expand All @@ -151,11 +152,11 @@ func TestPageUtil_GetNext(t *testing.T) {
ps := &testResponse{}
_ = json.NewDecoder(response.Body).Decode(ps)

nextPageUrl, err := GetNext(baseUrl, ps, getSomething)
nextPageUrl, err := GetNextWithContext(context.TODO(), baseUrl, ps, getSomething)
assert.Equal(t, "https://api.twilio.com/2010-04-01/Accounts/ACXX/Messages.json?From=9999999999&PageNumber=&To=4444444444&PageSize=2&Page=1&PageToken=PASMXX", nextPageUrl)
assert.Nil(t, err)

nextPageUrl, err = GetNext(baseUrl, nil, getSomething)
nextPageUrl, err = GetNextWithContext(context.TODO(), baseUrl, nil, getSomething)
assert.Empty(t, nextPageUrl)
assert.Nil(t, err)
}
Expand Down
41 changes: 29 additions & 12 deletions client/request_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
package client

import (
"context"
"net/http"
"net/url"
"os"
Expand All @@ -14,6 +15,13 @@ type RequestHandler struct {
Region string
}

type RequestHandlerWithContext struct {
Client BaseClient
Edge string
Region string
ctx context.Context
}

func NewRequestHandler(client BaseClient) *RequestHandler {
return &RequestHandler{
Client: client,
Expand All @@ -22,7 +30,16 @@ func NewRequestHandler(client BaseClient) *RequestHandler {
}
}

func (c *RequestHandler) sendRequest(method string, rawURL string, data url.Values,
func NewRequestHandlerWithContext(client BaseClient) *RequestHandlerWithContext {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure you need a separate struct for this honestly, just add a new method/methods on the RequestHandler and call it from everything.

return &RequestHandlerWithContext{
Client: client,
Edge: os.Getenv("TWILIO_EDGE"),
Region: os.Getenv("TWILIO_REGION"),
ctx: context.TODO(),
}
}

func (c *RequestHandlerWithContext) sendRequest(method string, rawURL string, data url.Values,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think what you want here is to have the first method here be a context.Context and then you can edit the methods below to call this instead.

headers map[string]interface{}, body ...byte) (*http.Response, error) {
parsedURL, err := c.BuildUrl(rawURL)
if err != nil {
Expand All @@ -32,7 +49,7 @@ func (c *RequestHandler) sendRequest(method string, rawURL string, data url.Valu
}

// BuildUrl builds the target host string taking into account region and edge configurations.
func (c *RequestHandler) BuildUrl(rawURL string) (string, error) {
func (c *RequestHandlerWithContext) BuildUrl(rawURL string) (string, error) {
u, err := url.Parse(rawURL)
if err != nil {
return "", err
Expand Down Expand Up @@ -82,22 +99,22 @@ func (c *RequestHandler) BuildUrl(rawURL string) (string, error) {
return u.String(), nil
}

func (c *RequestHandler) Post(path string, bodyData url.Values, headers map[string]interface{}, body ...byte) (*http.Response, error) {
return c.sendRequest(http.MethodPost, path, bodyData, headers, body...)
func (c *RequestHandlerWithContext) PostWithContext(ctx context.Context, path string, bodyData url.Values, headers map[string]interface{}, body ...byte) (*http.Response, error) {
return c.Client.SendRequestWithContext(ctx, http.MethodPost, path, bodyData, headers, body...)
}

func (c *RequestHandler) Put(path string, bodyData url.Values, headers map[string]interface{}, body ...byte) (*http.Response, error) {
return c.sendRequest(http.MethodPut, path, bodyData, headers, body...)
func (c *RequestHandlerWithContext) PutWithContext(ctx context.Context, path string, bodyData url.Values, headers map[string]interface{}, body ...byte) (*http.Response, error) {
return c.Client.SendRequestWithContext(ctx, http.MethodPut, path, bodyData, headers, body...)
}

func (c *RequestHandler) Patch(path string, bodyData url.Values, headers map[string]interface{}, body ...byte) (*http.Response, error) {
return c.sendRequest(http.MethodPatch, path, bodyData, headers, body...)
func (c *RequestHandlerWithContext) PatchWithContext(ctx context.Context, path string, bodyData url.Values, headers map[string]interface{}, body ...byte) (*http.Response, error) {
return c.Client.SendRequestWithContext(ctx, http.MethodPatch, path, bodyData, headers, body...)
}

func (c *RequestHandler) Get(path string, queryData url.Values, headers map[string]interface{}) (*http.Response, error) {
return c.sendRequest(http.MethodGet, path, queryData, headers)
func (c *RequestHandlerWithContext) GetWithContext(ctx context.Context, path string, queryData url.Values, headers map[string]interface{}) (*http.Response, error) {
return c.Client.SendRequestWithContext(ctx, http.MethodGet, path, queryData, headers)
}

func (c *RequestHandler) Delete(path string, queryData url.Values, headers map[string]interface{}) (*http.Response, error) {
return c.sendRequest(http.MethodDelete, path, queryData, headers)
func (c *RequestHandlerWithContext) DeleteWithContext(ctx context.Context, path string, queryData url.Values, headers map[string]interface{}) (*http.Response, error) {
return c.Client.SendRequestWithContext(ctx, http.MethodDelete, path, queryData, headers)
}
11 changes: 6 additions & 5 deletions client/request_handler_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package client_test

import (
"context"
"errors"
"net/http"
"net/http/httptest"
Expand All @@ -11,9 +12,9 @@ import (
"github.com/twilio/twilio-go/client"
)

func NewRequestHandler(accountSid string, authToken string) *client.RequestHandler {
func NewRequestHandler(accountSid string, authToken string) *client.RequestHandlerWithContext {
c := NewClient(accountSid, authToken)
return client.NewRequestHandler(c)
return client.NewRequestHandlerWithContext(c)
}

func TestRequestHandler_BuildUrlSetRegion(t *testing.T) {
Expand Down Expand Up @@ -62,7 +63,7 @@ func TestRequestHandler_BuildUrlInvalidCTLCharacter(t *testing.T) {
assert.Equal(t, parsedURL, "")
}

func assertAndGetURL(t *testing.T, requestHandler *client.RequestHandler, rawURL string) string {
func assertAndGetURL(t *testing.T, requestHandler *client.RequestHandlerWithContext, rawURL string) string {
parsedURL, err := requestHandler.BuildUrl(rawURL)
assert.Nil(t, err)
return parsedURL
Expand All @@ -83,7 +84,7 @@ func TestRequestHandler_SendGetRequest(t *testing.T) {
defer errorServer.Close()

requestHandler := NewRequestHandler("user", "pass")
resp, err := requestHandler.Get(errorServer.URL, nil, nil) //nolint:bodyclose
resp, err := requestHandler.GetWithContext(context.TODO(), errorServer.URL, nil, nil) //nolint:bodyclose
twilioError := err.(*client.TwilioRestError)
assert.Nil(t, resp)
assert.Equal(t, 400, twilioError.Status)
Expand All @@ -108,7 +109,7 @@ func TestRequestHandler_SendPostRequest(t *testing.T) {
defer errorServer.Close()

requestHandler := NewRequestHandler("user", "pass")
resp, err := requestHandler.Post(errorServer.URL, nil, nil) //nolint:bodyclose
resp, err := requestHandler.PostWithContext(context.TODO(), errorServer.URL, nil, nil) //nolint:bodyclose
twilioError := err.(*client.TwilioRestError)
assert.Nil(t, resp)
assert.Equal(t, 400, twilioError.Status)
Expand Down
Loading