Use passed-in context

Co-authored-by: Ryan Richard <richardry@vmware.com>
This commit is contained in:
Joshua Casey
2024-05-17 14:07:23 -05:00
parent a12a5f387a
commit 555b1c80e3
2 changed files with 67 additions and 15 deletions

View File

@@ -24,9 +24,9 @@ type TeamInfo struct {
}
type GitHubInterface interface {
GetUserInfo() (*UserInfo, error)
GetOrgMembership() ([]string, error)
GetTeamMembership(allowedOrganizations sets.Set[string]) ([]TeamInfo, error)
GetUserInfo(ctx context.Context) (*UserInfo, error)
GetOrgMembership(ctx context.Context) ([]string, error)
GetTeamMembership(ctx context.Context, allowedOrganizations sets.Set[string]) ([]TeamInfo, error)
}
type githubClient struct {
@@ -63,9 +63,9 @@ func NewGitHubClient(httpClient *http.Client, apiBaseURL, token string) (GitHubI
}
// GetUserInfo returns the "Login" and "ID" attributes of the logged-in user.
// TODO: where should context come from?
func (g *githubClient) GetUserInfo() (*UserInfo, error) {
user, response, err := g.client.Users.Get(context.Background(), emptyUserMeansTheAuthenticatedUser)
// TODO: should we check ID and Login for nil?
func (g *githubClient) GetUserInfo(ctx context.Context) (*UserInfo, error) {
user, response, err := g.client.Users.Get(ctx, emptyUserMeansTheAuthenticatedUser)
if err != nil {
return nil, fmt.Errorf("error fetching authenticated user: %w", err)
}
@@ -89,16 +89,15 @@ func (g *githubClient) GetUserInfo() (*UserInfo, error) {
}
// GetOrgMembership returns an array of the "Login" attributes for all organizations to which the authenticated user belongs.
// TODO: where should context come from?
// TODO: what happens if login is nil?
// TODO: what is a good page size?
func (g *githubClient) GetOrgMembership() ([]string, error) {
func (g *githubClient) GetOrgMembership(ctx context.Context) ([]string, error) {
organizationLoginStrings := make([]string, 0)
opt := &github.ListOptions{PerPage: 10}
// get all pages of results
for {
organizationResults, response, err := g.client.Organizations.List(context.Background(), emptyUserMeansTheAuthenticatedUser, opt)
organizationResults, response, err := g.client.Organizations.List(ctx, emptyUserMeansTheAuthenticatedUser, opt)
if err != nil {
return nil, fmt.Errorf("error fetching organizations for authenticated user: %w", err)
}
@@ -122,16 +121,15 @@ func isOrgAllowed(allowedOrganizations sets.Set[string], login string) bool {
// GetTeamMembership returns a description of each team to which the authenticated user belongs.
// If allowedOrganizations is not empty, will filter the results to only those teams which belong to the allowed organizations.
// Parent teams will also be returned.
// TODO: where should context come from?
// TODO: what happens if org or login or id are nil?
// TODO: what is a good page size?
func (g *githubClient) GetTeamMembership(allowedOrganizations sets.Set[string]) ([]TeamInfo, error) {
func (g *githubClient) GetTeamMembership(ctx context.Context, allowedOrganizations sets.Set[string]) ([]TeamInfo, error) {
teamInfos := make([]TeamInfo, 0)
opt := &github.ListOptions{PerPage: 10}
// get all pages of results
for {
teamsResults, response, err := g.client.Teams.ListUserTeams(context.Background(), opt)
teamsResults, response, err := g.client.Teams.ListUserTeams(ctx, opt)
if err != nil {
return nil, fmt.Errorf("error fetching team membership for authenticated user: %w", err)
}

View File

@@ -1,6 +1,7 @@
package githubclient
import (
"context"
"net/http"
"strings"
"testing"
@@ -126,6 +127,7 @@ func TestGetUser(t *testing.T) {
name string
httpClient *http.Client
token string
ctx context.Context
wantErr string
wantUserInfo UserInfo
}{
@@ -191,6 +193,17 @@ func TestGetUser(t *testing.T) {
token: "does-this-token-work",
wantErr: `the "ID" attribute is missing for authenticated user`,
},
{
name: "passes the context parameter into the API call",
token: "some-token",
httpClient: mock.NewMockedHTTPClient(),
ctx: func() context.Context {
canceledCtx, cancel := context.WithCancel(context.Background())
cancel()
return canceledCtx
}(),
wantErr: "error fetching authenticated user: context canceled",
},
{
name: "returns errors from the API",
httpClient: mock.NewMockedHTTPClient(
@@ -216,7 +229,13 @@ func TestGetUser(t *testing.T) {
githubClient := &githubClient{
client: github.NewClient(test.httpClient).WithAuthToken(test.token),
}
actual, err := githubClient.GetUserInfo()
ctx := context.Background()
if test.ctx != nil {
ctx = test.ctx
}
actual, err := githubClient.GetUserInfo(ctx)
if test.wantErr != "" {
rt, ok := test.httpClient.Transport.(*mock.EnforceHostRoundTripper)
require.True(t, ok)
@@ -238,6 +257,7 @@ func TestGetOrgMembership(t *testing.T) {
name string
httpClient *http.Client
token string
ctx context.Context
wantErr string
wantOrgs []string
}{
@@ -292,6 +312,17 @@ func TestGetOrgMembership(t *testing.T) {
token: "does-this-token-work",
wantOrgs: []string{"some-org-to-which-the-authenticated-user-belongs"},
},
{
name: "passes the context parameter into the API call",
token: "some-token",
httpClient: mock.NewMockedHTTPClient(),
ctx: func() context.Context {
canceledCtx, cancel := context.WithCancel(context.Background())
cancel()
return canceledCtx
}(),
wantErr: "error fetching organizations for authenticated user: context canceled",
},
{
name: "returns errors from the API",
httpClient: mock.NewMockedHTTPClient(
@@ -317,7 +348,13 @@ func TestGetOrgMembership(t *testing.T) {
githubClient := &githubClient{
client: github.NewClient(test.httpClient).WithAuthToken(test.token),
}
actual, err := githubClient.GetOrgMembership()
ctx := context.Background()
if test.ctx != nil {
ctx = test.ctx
}
actual, err := githubClient.GetOrgMembership(ctx)
if test.wantErr != "" {
rt, ok := test.httpClient.Transport.(*mock.EnforceHostRoundTripper)
require.True(t, ok)
@@ -339,6 +376,7 @@ func TestGetTeamMembership(t *testing.T) {
name string
httpClient *http.Client
token string
ctx context.Context
allowedOrganizations []string
wantErr string
wantTeams []TeamInfo
@@ -647,6 +685,17 @@ func TestGetTeamMembership(t *testing.T) {
},
},
},
{
name: "passes the context parameter into the API call",
token: "some-token",
httpClient: mock.NewMockedHTTPClient(),
ctx: func() context.Context {
canceledCtx, cancel := context.WithCancel(context.Background())
cancel()
return canceledCtx
}(),
wantErr: "error fetching team membership for authenticated user: context canceled",
},
{
name: "returns errors from the API",
httpClient: mock.NewMockedHTTPClient(
@@ -673,7 +722,12 @@ func TestGetTeamMembership(t *testing.T) {
client: github.NewClient(test.httpClient).WithAuthToken(test.token),
}
actual, err := githubClient.GetTeamMembership(sets.New[string](test.allowedOrganizations...))
ctx := context.Background()
if test.ctx != nil {
ctx = test.ctx
}
actual, err := githubClient.GetTeamMembership(ctx, sets.New[string](test.allowedOrganizations...))
if test.wantErr != "" {
rt, ok := test.httpClient.Transport.(*mock.EnforceHostRoundTripper)
require.True(t, ok)