SUP-819 Retrieve org id only once in provider configuration (#263)

* SUP-819 Retrieve org id only once in provider configuration

* SUP-819 Fix return types in util_test.go

* SUP-819 Fix util_test.go to properly test GetOrganizationID

buildkite/client.go

@@ -14,11 +14,12 @@ import (
 
 // Client can be used to interact with the Buildkite API
 type Client struct {
-	graphql      *graphql.Client
-	genqlient    genqlient.Client
-	http         *http.Client
-	organization string
-	restUrl      string
+	graphql        *graphql.Client
+	genqlient      genqlient.Client
+	http           *http.Client
+	organization   string
+	organizationId string
+	restUrl        string
 }
 
 type clientConfig struct {
@@ -35,7 +36,7 @@ type headerRoundTripper struct {
 }
 
 // NewClient creates a client to use for interacting with the Buildkite API
-func NewClient(config *clientConfig) *Client {
+func NewClient(config *clientConfig) (*Client, error) {
 
 	// Setup a HTTP Client that can be used by all REST and graphql API calls,
 	// with suitable headers for authentication and user agent identification
@@ -49,13 +50,21 @@ func NewClient(config *clientConfig) *Client {
 		Transport: rt,
 	}
 
-	return &Client{
-		graphql:      graphql.NewClient(config.graphqlURL, httpClient),
-		genqlient:    genqlient.NewClient(config.graphqlURL, httpClient),
-		http:         httpClient,
-		organization: config.org,
-		restUrl:      config.restURL,
+	graphqlClient := graphql.NewClient(config.graphqlURL, httpClient)
+	orgId, err := GetOrganizationID(config.org, graphqlClient)
+
+	if err != nil {
+		return nil, err
 	}
+
+	return &Client{
+		graphql:        graphqlClient,
+		genqlient:      genqlient.NewClient(config.graphqlURL, httpClient),
+		http:           httpClient,
+		organization:   config.org,
+		organizationId: orgId,
+		restUrl:        config.restURL,
+	}, nil
 }
 
 func newHeaderRoundTripper(next http.RoundTripper, header http.Header) *headerRoundTripper {

buildkite/provider.go

@@ -80,6 +80,6 @@ func providerConfigure(userAgent string) func(d *schema.ResourceData) (interface
 			userAgent:  userAgent,
 		}
 
-		return NewClient(config), nil
+		return NewClient(config)
 	}
 }

buildkite/resource_agent_token.go

@@ -49,10 +49,6 @@ func resourceAgentToken() *schema.Resource {
 // CreateToken creates a Buildkite agent token
 func CreateToken(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
 	client := m.(*Client)
-	id, err := GetOrganizationID(client.organization, client.graphql)
-	if err != nil {
-		return diag.FromErr(err)
-	}
 
 	var mutation struct {
 		AgentTokenCreate struct {
@@ -63,11 +59,11 @@ func CreateToken(ctx context.Context, d *schema.ResourceData, m interface{}) dia
 	}
 
 	vars := map[string]interface{}{
-		"org":  id,
+		"org":  client.organizationId,
 		"desc": graphql.String(d.Get("description").(string)),
 	}
 
-	err = client.graphql.Mutate(context.Background(), &mutation, vars)
+	err := client.graphql.Mutate(context.Background(), &mutation, vars)
 	if err != nil {
 		return diag.FromErr(err)
 	}

buildkite/resource_pipeline.go

@@ -318,10 +318,7 @@ func resourcePipeline() *schema.Resource {
 // CreatePipeline creates a Buildkite pipeline
 func CreatePipeline(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
 	client := m.(*Client)
-	orgID, err := GetOrganizationID(client.organization, client.graphql)
-	if err != nil {
-		return diag.FromErr(err)
-	}
+	var err error
 
 	teamPipelines := getTeamPipelinesFromSchema(d)
 	var mutation struct {
@@ -352,7 +349,7 @@ func CreatePipeline(ctx context.Context, d *schema.ResourceData, m interface{})
 		"maximum_timeout_in_minutes":               graphql.Int(d.Get("maximum_timeout_in_minutes").(int)),
 		"desc":                                     graphql.String(d.Get("description").(string)),
 		"name":                                     graphql.String(d.Get("name").(string)),
-		"org":                                      orgID,
+		"org":                                      client.organizationId,
 		"repository_url":                           graphql.String(d.Get("repository").(string)),
 		"skip_intermediate_builds":                 graphql.Boolean(d.Get("skip_intermediate_builds").(bool)),
 		"skip_intermediate_builds_branch_filter":   graphql.String(d.Get("skip_intermediate_builds_branch_filter").(string)),

buildkite/resource_team.go

@@ -98,10 +98,6 @@ func resourceTeam() *schema.Resource {
 // CreateTeam creates a Buildkite team
 func CreateTeam(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
 	client := m.(*Client)
-	id, err := GetOrganizationID(client.organization, client.graphql)
-	if err != nil {
-		return diag.FromErr(err)
-	}
 
 	var mutation struct {
 		TeamCreate struct {
@@ -112,7 +108,7 @@ func CreateTeam(ctx context.Context, d *schema.ResourceData, m interface{}) diag
 	}
 
 	vars := map[string]interface{}{
-		"org":                          id,
+		"org":                          client.organizationId,
 		"name":                         graphql.String(d.Get("name").(string)),
 		"desc":                         graphql.String(d.Get("description").(string)),
 		"privacy":                      TeamPrivacy(d.Get("privacy").(string)),
@@ -121,7 +117,7 @@ func CreateTeam(ctx context.Context, d *schema.ResourceData, m interface{}) diag
 		"members_can_create_pipelines": graphql.Boolean(d.Get("members_can_create_pipelines").(bool)),
 	}
 
-	err = client.graphql.Mutate(context.Background(), &mutation, vars)
+	err := client.graphql.Mutate(context.Background(), &mutation, vars)
 	if err != nil {
 		return diag.FromErr(err)
 	}

buildkite/util_test.go

@@ -16,13 +16,12 @@ func TestGetOrganizationIDMissing(t *testing.T) {
 		userAgent:  "test-user-agent",
 	}
 
-	client := NewClient(config)
-
-	id, err := GetOrganizationID(slug, client.graphql)
+	// NewClient calls GetOrganizationId so we can test the output
+	client, err := NewClient(config)
 	if err == nil {
 		t.Fatalf("err: %s", err)
 	}
-	if id != "" {
+	if client != nil {
 		t.Fatalf("Nonexistent organization found")
 	}
 }